Re: [Qemu-devel] [RFC PATCH 0/5] hotplug: fix premature rebinding of VFIO devices to host

[Andrea Bolognani]

On Thu, 2017-06-29 at 15:59 -0500, Michael Roth wrote:
> > > Patches 1-4 address 1) by deferring rebinding of a hostdev to the host 
> > > driver
> > > until all the devices in the group have been detached, at which point all
> > > the hostdevs are rebound as a group. Until that point, the devices are 
> > > traced
> > > by the drvManager's inactiveList in a similar manner to hostdevs that are
> > > assigned to VFIO via the nodedev-detach interface.
> > 
> > What happens if libvirtd is restarted during this period? Is the
> > inactiveList rebuilt with all the info necessary to assure that the
> > nodedev-reattach does/doesn't happen (as appropriate) for all devices?
> 
> Hmm, good question.
> 
> The Unbindable() check only needs to know that nothing in the activeList
> belongs to the group we're checking, and that list at least seems to get
> rebuilt appropriately on restart.
> 
> But the Unbind() relies on inactiveList and the behavior there is what
> nodedev-detach currently does, which is to add it to inactive list while
> libvirtd is running, and then just forget about it when libvirtd restarts.
> For nodedev-detach it's fine since virHostdevPreparePCIDevices() re-adds
> it as needed in the device-attach path. But yah, for this purpose it ends
> up losing track of hostdevs that are still pending rebind to the host, and
> that means some devices may not get rebound at the appropriate time if
> there was a libvirtd restart.
> 
> Unlike with device-attach, we can't just re-add it on-demand because we
> actually need to know whether or not it was previously in the list. So
> I think we'd need to add some persistent state to track this. Will look
> into adding handling for that.

FWIW last time a tried to attack this issue[1] I got pretty
much as far as this, eg. the code worked as intended but
restarting libvirtd would result in an inconsistent state
which prevented you from performing some operations.

Unfortunately I got sidetracked by other work and stopped
just short of implementing a way to persist the relevant
information on disk :(


[1] ~1.5 years ago, according to git log
-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [Qemu-devel] [PATCH] hw/ppc/spapr.c: do not adjust rma_size with KVM RADIX in ppc_spapr_init

[David Gibson]

On Wed, Jun 28, 2017 at 04:47:31PM -0300, Daniel Henrique Barboza wrote:
> In ppc_spapr_init when setting rma_size we have the following verification:
> 
> if (rma_alloc_size && (rma_alloc_size < node0_size)) {
> spapr->rma_size = rma_alloc_size;
> } else {
> spapr->rma_size = node0_size;
> 
> /* With KVM, we don't actually know whether KVM supports an
>  * unbounded RMA (PR KVM) or is limited by the hash table size
>  * (HV KVM using VRMA), so we always assume the latter
>  *
>  * In that case, we also limit the initial allocations for RTAS
>  * etc... to 256M since we have no way to know what the VRMA size
>  * is going to be as it depends on the size of the hash table
>  * isn't determined yet.
>  */
> if (kvm_enabled()) {
> spapr->vrma_adjust = 1;
> spapr->rma_size = MIN(spapr->rma_size, 0x1000);
> }
> 
> This code (and the comment that precedes it) is taking constraints and 
> conditions
> related to KVM HPT guests and filtering them with "if (kvm_enabled())". Note 
> that
> this also means that, for KVM RADIX guests, we'll change rma_size and set the
> vrma_adjust flag as well.
> 
> The flag vrma_adjust is used inside 'spapr_setup_hpt_and_vrma', which in turn 
> is
> called from ppc_spapr_reset as follows:
> 
> if (kvm_enabled() && kvmppc_has_cap_mmu_radix()) {
> /* If using KVM with radix mode available, VCPUs can be started
>  * without a HPT because KVM will start them in radix mode.
>  * Set the GR bit in PATB so that we know there is no HPT. */
> spapr->patb_entry = PATBE1_GR;
> } else {
> spapr_setup_hpt_and_vrma(spapr);
> }
> 
> In short, when running a KVM HPT guest, rma_size is shrinked, the flag 
> vrma_adjust
> is set and later on spapr_setup_hpt_and_vrma() is called to make the proper
> adjustments. When running a KVM RADIX guest no post adjustment is made, 
> leaving
> rma_size with the value MIN(spapr->rma_size, 0x1000).
> 
> This changed rma_size value is causing the code to populate more memory nodes
> in 'spapr_populate_memory', which in turn results in differences in the memory
> layout at SLOF init (alloc_top and rmo_top). At first this isn't causing bugs 
> or
> guest misbehavior in case of KVM RADIX - the problem is that this is happening
> due to KVM HPT code.
> 
> This patch changes the if conditional inside ppc_spapr_init to:
> 
> if (kvm_enabled() && !kvmppc_has_cap_mmu_radix()) {
> spapr->vrma_adjust = 1;
> spapr->rma_size = MIN(spapr->rma_size, 0x1000);
> }
> 
> To restrict the rma changes only to KVM HPT guests. If we need to do
> adjustments for RADIX we should either do it explicitly in its own code
> or make it clearer that a common code applies to both HPT and RADIX.
> 
> Signed-off-by: Daniel Henrique Barboza 

This doesn't seem right to me, on a few levels.

First, if the guest is RPT, then AFAIK, the whole concept of an RMA is
basically meaningless - so we should be reflecting that throughout not
just removing one adjustment to it.

We probably need some cleanup of the existing code here - at the
moment these RMA adjustments make guest-visible changes based on
whether we're KVM or not, which is not an ideal situation at all.

> ---
>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7d9af75..117ea9d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2164,7 +2164,7 @@ static void ppc_spapr_init(MachineState *machine)
>   * is going to be as it depends on the size of the hash table
>   * isn't determined yet.
>   */
> -if (kvm_enabled()) {
> +if (kvm_enabled() && !kvmppc_has_cap_mmu_radix()) {

In addition, this looks like the wrong test.  This tests if KVM is
_capable_ of doing radix, not if we actually _are_ doing radix.  At
the moment an RPT host can't run an HPT guest, but I believe we're
planning to change that at some point.

>  spapr->vrma_adjust = 1;
>  spapr->rma_size = MIN(spapr->rma_size, 0x1000);
>  }

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v6 04/10] migration: let MigrationState be a qdev

[Peter Xu]

On Fri, Jun 30, 2017 at 11:03:21AM +0800, Peter Xu wrote:
> On Fri, Jun 30, 2017 at 04:18:56AM +0200, Max Reitz wrote:
> > On 2017-06-27 06:10, Peter Xu wrote:
> > > Let the old man "MigrationState" join the object family. Direct benefit
> > > is that we can start to use all the property features derived from
> > > current QDev, like: HW_COMPAT_* bits, command line setup for migration
> > > parameters (so will never need to set them up each time using HMP/QMP,
> > > this is really, really attractive for test writters), etc.
> > > 
> > > I see no reason to disallow this happen yet. So let's start from this
> > > one, to see whether it would be anything good.
> > > 
> > > Now we init the MigrationState struct statically in main() to make sure
> > > it's initialized after global properties are applied, since we'll use
> > > them during creation of the object.
> > > 
> > > No functional change at all.
> > 
> > Evidently not quite right because this breaks iotest 055.
> > 
> > Condensed test case:
> > 
> > $ ./qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 64M
> > Formatting 'foo.vmdk', fmt=vmdk size=67108864 compat6=off
> > hwversion=undefined subformat=streamOptimized
> > $ x86_64-softmmu/qemu-system-x86_64 -drive if=none,file=foo.vmdk
> > qemu-system-x86_64: ./migration/migration.c:114: migrate_get_current:
> > Assertion `current_migration' failed.
> > [1]15453 abort (core dumped)  x86_64-softmmu/qemu-system-x86_64
> > -drive if=none,file=foo.vmdk
> > 
> > (It just worked before this patch.)
> 
> Sorry. Will have a look.

Hello, Max,

The assertion is caused by migrate_add_blocker() called before
initialization of migration object. I'll fix it.

But even with a fix (so I can pass 055 now), I still cannot pass some
of the other tests. Errors I got:

  https://pastebin.com/ACqbXAYd

I am not familiar with iotests. Is above usual? Looks like it still
includes 3 failures, and some output mismatch.

-- 
Peter Xu

[Qemu-devel] [PATCH v2] intel_iommu: fix migration breakage on mr switch

[Peter Xu]

Migration is broken after the vfio integration work:

qemu-kvm: AHCI: Failed to start FIS receive engine: bad FIS receive buffer 
address
qemu-kvm: Failed to load ich9_ahci:ahci
qemu-kvm: error while loading state for instance 0x0 of device 
':00:1f.2/ich9_ahci'
qemu-kvm: load of migration failed: Operation not permitted

The problem is that vfio work introduced dynamic memory region
switching (actually it is also used for future PT mode), and this memory
region layout is not properly delivered to destination when migration
happens. Solution is to rebuild the layout in post_load.

Bug: https://bugzilla.redhat.com/show_bug.cgi?id=1459906
Fixes: 558e0024 ("intel_iommu: allow dynamic switch of IOMMU region")
Reviewed-by: Jason Wang 
Signed-off-by: Peter Xu 
---
v2: tune comment [mst]
---
 hw/i386/intel_iommu.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index a9b59bd..05d9935 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2332,11 +2332,26 @@ static void vtd_iommu_notify_flag_changed(MemoryRegion 
*iommu,
 }
 }
 
+static int vtd_post_load(void *opaque, int version_id)
+{
+IntelIOMMUState *iommu = opaque;
+
+/*
+ * Memory regions are dynamically turned on/off depending on
+ * context entry configurations from the guest. After migration,
+ * we need to make sure the memory regions are still correct.
+ */
+vtd_switch_address_space_all(iommu);
+
+return 0;
+}
+
 static const VMStateDescription vtd_vmstate = {
 .name = "iommu-intel",
 .version_id = 1,
 .minimum_version_id = 1,
 .priority = MIG_PRI_IOMMU,
+.post_load = vtd_post_load,
 .fields = (VMStateField[]) {
 VMSTATE_UINT64(root, IntelIOMMUState),
 VMSTATE_UINT64(intr_root, IntelIOMMUState),
-- 
2.7.4

Re: [Qemu-devel] [PATCH RFC 0/6] q35: add acpi pci hotplug support

[Marcel Apfelbaum]

On 30/06/2017 2:17, Michael S. Tsirkin wrote:

On Fri, Jun 30, 2017 at 12:55:56AM +0300, Aleksandr Bezzubikov wrote:

The series adds hotplug support to legacy PCI buses for Q35 machines.
The ACPI hotplug code is emitted if at least one legacy pci-bridge is present.

This series is mostly based on past Marcel's series
https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg05681.html,
but rebased on current master with some minor changes according to current 
codebase.

ACPI code emission approach used in this series can be called "static",
because it checkswhether a bridge exists only at initial DSDT generation moment.
The main goal is to enable AML PCI hotplug-related code to be generated 
dynamically.
In other words, the bridge plugged in - a new acpi definition block is
loaded (using LoadTable method).
This is necessary for PCIE-PCI bridge hotplugging feature.




Hi Michael,
Thank you for looking into this.


I wonder whether we really need ACPI hotplug.
Most modern systems are limited to native PCIE.



I was under impression that ACPI hotplug will still remain
for memory/cpu hotplug, but I might be wrong. If this
is the case (ACPI hotplug is used for other subsystems),
I don't see why modern system would disable the PCI APCI hoptplug.
(I am not saying PCIe hotplug is not preferred.)

And if the modern systems are limited to native PCIe
we might have a bigger problem since the QEMU default
x86 machine is PCI based and we don't have PCIe obviously...
maybe is this the right time to switch to Q35 as the default machine?
(I am aware it should be a different discussion)



I do understand the need to add PCI devices sometimes,
but maybe we can find a way to do this with native hotplug.

For example, how about the following approach


- PCIE downstream port - X - PCIE-to-PCI bridge - PCI device



It can be a solution, yes, but then we would limit (seriously)
the number of PCI devices we can add. It will not be a problem
if we will continue to keep Q35 for "modern systems" only machine
and keep PC around as the default machine. However adding full support
for pci-bridge will allow us to switch to Q35 and use it
as the default machine (sometime sooner) since it would
be easier to map older configurations.



By hotplugging the bridge+device combination into a downstream
port (point X) we can potentially make devices enumerate
properly.



It may work, yes, Alexandr will you be willing to try it?


This might cause some issues with IO port assignment (uses 4K
io port space due to bridge aperture limitations)
but maybe we do not need so many legacy PCI devices -
people who do can simply use piix.



IO port assignment issue can be solved by using non standard
IO port space, some OSes can actually deal with it (I think),
however it will not solve the limitation of the number of
pci devices we can have, since each device (PCIe or not) will be
under a different bus we will have 256 PCI devices max.
We can use multi-functions, but then the hot-plug granularity
goes away.



For this to work we need a way to create bridge instance
that is invisible to the guest. There is already a
way to do this for multifunction devices:

create bridge in function != 0
attach device
then create a dummy function 0

Inelegant - it would be cleaner to support this for function 0
as well - but should allow you to test the idea directly.



The benefit of this project is to  make Q35 a "wider" machine
that would include all prev QEMU devices and will facilitate
the transition from the pc to q35 machine.

So for the modern systems not supporting PCI ACPI hotplug
we don't need pci-bridges anyway, but for the older ones
the ACPI code of the pci-bridge will be loaded into the
ACPI namespace only if a pci-bridge is actually hot-plugged.

That being said, if we decide that q35 will be used for
modern systems only and the pc machine will remain the
default for the next years, we may try to bundle
the pci-bridge with the device/s behind it.

Thanks,
Marcel





Aleksandr Bezzubikov (6):
   hw/acpi: remove dead acpi code
   hw/acpi: simplify dsdt building code
   hw/acpi: fix pcihp io initialization
   hw/acpi: prepare pci hotplug IO for ich9
   hw/acpi: extend acpi pci hotplug support for pci express
   hw/ich9: enable acpi pci hotplug

  hw/acpi/ich9.c |  31 +
  hw/i386/acpi-build.c   | 116 -
  hw/isa/lpc_ich9.c  |  12 +
  include/hw/acpi/ich9.h |   4 ++
  include/hw/i386/pc.h   |   7 ++-
  5 files changed, 111 insertions(+), 59 deletions(-)

--
2.7.4

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1

[David Gibson]

On Thu, Jun 29, 2017 at 03:42:03PM +1000, Suraj Jitindar Singh wrote:
> On Thu, 2017-06-29 at 15:37 +1000, Suraj Jitindar Singh wrote:
> > On Wed, 2017-06-28 at 18:41 +0200, Greg Kurz wrote:
> > > On Wed, 28 Jun 2017 18:18:06 +0200
> > > Laurent Vivier  wrote:
> > > 
> > > > On 28/06/2017 13:59, Greg Kurz wrote:
> > > > > On Wed, 28 Jun 2017 12:23:06 +0200
> > > > > Cédric Le Goater  wrote:
> > > > >   
> > > > > > On 06/28/2017 11:18 AM, Laurent Vivier wrote:  
> > > > > > > On 28/06/2017 11:11, Cédric Le Goater wrote:
> > > > > > > > On 06/28/2017 10:18 AM, David Gibson wrote:
> > > > > > > > > On Wed, Jun 28, 2017 at 09:09:24AM +0200, Thomas Huth
> > > > > > > > > wrote:
> > > > > > > > > > On 28.06.2017 03:42, jos...@linux.vnet.ibm.com
> > > > > > > > > > wrote:
> > > > > > > > > > > On Fri, Jun 23, 2017 at 04:10:55PM +0200, Laurent
> > > > > > > > > > > Vivier wrote:
> > > > > > > > > > > > On 23/06/2017 11:21, David Gibson wrote:
> > > > > > > > > > > > > On Thu, Jun 22, 2017 at 01:31:24PM +0200,
> > > > > > > > > > > > > Thomas
> > > > > > > > > > > > > Huth wrote:
> > > > > > > > > > > > > > On 22.06.2017 13:26, Laurent Vivier
> > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > CPU_POWERPC_POWER9_DD1 is 0x004E0100, so
> > > > > > > > > > > > > > > this
> > > > > > > > > > > > > > > is the POWER9 v1.0.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > When we run qemu on a POWER9 DD1 host, we
> > > > > > > > > > > > > > > must use either
> > > > > > > > > > > > > > > "-cpu host" or "-cpu POWER9", but in the
> > > > > > > > > > > > > > > latter case it fails with
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Unable to find sPAPR CPU Core
> > > > > > > > > > > > > > > definition
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > because POWER9 DD1 doesn't appear in the
> > > > > > > > > > > > > > > list
> > > > > > > > > > > > > > > of known CPUs.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > This patch fixes this by defining
> > > > > > > > > > > > > > > POWER9_v1.0
> > > > > > > > > > > > > > > with POWER9 DD1
> > > > > > > > > > > > > > > PVR instead of CPU_POWERPC_POWER9_BASE.
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > Signed-off-by: Laurent Vivier  > > > > > > > > > > > > > > at
> > > > > > > > > > > > > > > .com>
> > > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > >  target/ppc/cpu-models.c | 2 +-
> > > > > > > > > > > > > > >  1 file changed, 1 insertion(+), 1
> > > > > > > > > > > > > > > deletion(-
> > > > > > > > > > > > > > > )
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > diff --git a/target/ppc/cpu-models.c
> > > > > > > > > > > > > > > b/target/ppc/cpu-models.c
> > > > > > > > > > > > > > > index 4d3e635..a22363c 100644
> > > > > > > > > > > > > > > --- a/target/ppc/cpu-models.c
> > > > > > > > > > > > > > > +++ b/target/ppc/cpu-models.c
> > > > > > > > > > > > > > > @@ -1144,7 +1144,7 @@
> > > > > > > > > > > > > > >  POWERPC_DEF("970_v2.2",  CPU_POWER
> > > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > > _970_v22,970,
> > > > > > > > > > > > > > >  "PowerPC 970 v2.2")
> > > > > > > > > > > > > > >  
> > > > > > > > > > > > > > > -POWERPC_DEF("POWER9_v1.0",   CPU_POWER
> > > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > > _POWER9_BASE,POWER9,
> > > > > > > > > > > > > > > +POWERPC_DEF("POWER9_v1.0",   CPU_POWER
> > > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > > _POWER9_DD1, POWER9,
> > > > > > > > > > > > > > >  "POWER9 v1.0")
> > > > > > > > > > > > > > >  
> > > > > > > > > > > > > > >  POWERPC_DEF("970fx_v1.0",CPU_POWER
> > > > > > > > > > > > > > > PC
> > > > > > > > > > > > > > > _970FX_v10,  970,
> > > > > > > > > > > > > > >    
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > I think this also makes sense for running in
> > > > > > > > > > > > > > TCG mode to get a valid
> > > > > > > > > > > > > > real PVR there.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > I'm not so convinced.
> > > > > > > > > > > > > 
> > > > > > > > > > > > > IIUC, this will make TCG default (for now) to a
> > > > > > > > > > > > > DD1 POWER9.  That's a)
> > > > > > > > > > > > > probably not what anyone wants - who'd select a
> > > > > > > > > > > > > buggy prototype and b)
> > > > > > > > > > > > > not accurate - TCG does not implement DD1's
> > > > > > > > > > > > > bugs.
> > > > > > > > > > > > 
> > > > > > > > > > > > According to the POWER8 user manual (I didn't
> > > > > > > > > > > > fine
> > > > > > > > > > > > the POWER9 one):
> > > > > > > > > > > > 
> > > > > > > > > > > > "3.6.3.1 Processor Version Register (PVR)
> > > > > > > > > > > > 
> > > > > > > > > > > > The processor revision level (PVR[16:31]) starts
> > > > > > > > > > > > at
> > > > > > > > > > > > x‘0100’, indicating
> > > > > > > >

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1

[David Gibson]

On Wed, Jun 28, 2017 at 06:18:06PM +0200, Laurent Vivier wrote:
> On 28/06/2017 13:59, Greg Kurz wrote:
> > On Wed, 28 Jun 2017 12:23:06 +0200
> > Cédric Le Goater  wrote:
> > 
> >> On 06/28/2017 11:18 AM, Laurent Vivier wrote:
> >>> On 28/06/2017 11:11, Cédric Le Goater wrote:  
>  On 06/28/2017 10:18 AM, David Gibson wrote:  
> > On Wed, Jun 28, 2017 at 09:09:24AM +0200, Thomas Huth wrote:  
> >> On 28.06.2017 03:42, jos...@linux.vnet.ibm.com wrote:  
> >>> On Fri, Jun 23, 2017 at 04:10:55PM +0200, Laurent Vivier wrote:  
>  On 23/06/2017 11:21, David Gibson wrote:  
> > On Thu, Jun 22, 2017 at 01:31:24PM +0200, Thomas Huth wrote:  
> >> On 22.06.2017 13:26, Laurent Vivier wrote:  
> >>> CPU_POWERPC_POWER9_DD1 is 0x004E0100, so this is the POWER9 v1.0.
> >>>
> >>> When we run qemu on a POWER9 DD1 host, we must use either
> >>> "-cpu host" or "-cpu POWER9", but in the latter case it fails with
> >>>
> >>> Unable to find sPAPR CPU Core definition
> >>>
> >>> because POWER9 DD1 doesn't appear in the list of known CPUs.
> >>>
> >>> This patch fixes this by defining POWER9_v1.0 with POWER9 DD1
> >>> PVR instead of CPU_POWERPC_POWER9_BASE.
> >>>
> >>> Signed-off-by: Laurent Vivier 
> >>> ---
> >>>  target/ppc/cpu-models.c | 2 +-
> >>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
> >>> index 4d3e635..a22363c 100644
> >>> --- a/target/ppc/cpu-models.c
> >>> +++ b/target/ppc/cpu-models.c
> >>> @@ -1144,7 +1144,7 @@
> >>>  POWERPC_DEF("970_v2.2",  CPU_POWERPC_970_v22,
> >>> 970,
> >>>  "PowerPC 970 v2.2")
> >>>  
> >>> -POWERPC_DEF("POWER9_v1.0",   CPU_POWERPC_POWER9_BASE,
> >>> POWER9,
> >>> +POWERPC_DEF("POWER9_v1.0",   CPU_POWERPC_POWER9_DD1, 
> >>> POWER9,
> >>>  "POWER9 v1.0")
> >>>  
> >>>  POWERPC_DEF("970fx_v1.0",CPU_POWERPC_970FX_v10,  
> >>> 970,
> >>>  
> >>
> >> I think this also makes sense for running in TCG mode to get a 
> >> valid
> >> real PVR there.  
> >
> > I'm not so convinced.
> >
> > IIUC, this will make TCG default (for now) to a DD1 POWER9.  That's 
> > a)
> > probably not what anyone wants - who'd select a buggy prototype and 
> > b)
> > not accurate - TCG does not implement DD1's bugs.  
> 
>  According to the POWER8 user manual (I didn't fine the POWER9 one):
> 
>  "3.6.3.1 Processor Version Register (PVR)
> 
>  The processor revision level (PVR[16:31]) starts at x‘0100’, 
>  indicating
>  revision ‘1.0’. As revisions are made, bits [29:31] will indicate 
>  minor
>  revisions. Similarly, bits [20:23] indicate major changes."
> 
>  POWER9 DD1 PVR is 0x004E0100, so this is really version 1.0 of the 
>  POWER9.
> 
>  Perhaps we can define POWER9_v1.0 as CPU_POWERPC_POWER9_DD1, and
>  introduce a POWER9_v0.0 set to CPU_POWERPC_POWER9_BASE and define it 
>  as
>  the default one?  
> >>>
> >>> I like the suggestion to set a v0.0 to CPU_POWERPC_POWER9_BASE. But, I
> >>> think we could have only that option, removing the
> >>> CPU_POWERPC_POWER9_DD1 entry.  
> >> I really dislike the idea of having a CPU called "v0.0" ... we do not
> >> have this for any other CPU generation, and it sounds like it could be
> >> very confusing for the users (you'd need to document somewhere what the
> >> v0.0 exactly means). If we really want to go this way, I think we 
> >> should
> >> name it "POWER9-generic" or "PowerISA-3.0" or something similar 
> >> instead.
> >>
> >> Or does somebody already know the exact PVR for DD2? If so, we could
> >> simply add a POWER9_v2.0 CPU already and let the POWER9 alias point to
> >> that version instead.  
> >
> > Yes, I think that's a better idea.  I don't know the DD2 PVR, but I'm
> > pretty sure we should be able to find out from someone at IBM.
> >
> > I've CCed Sam & Suraj - can you ask Mikey or someone what the PVR
> > value for DD2.0 will be?  
> 
>  I would expect something like :
> 
>   0x200D10498000UL; /* P9 Nimbus DD2.0 */  
> >>>
> >>>
> >>> I would expect something like 0x004E.  
> >>
> >> ah yes, I am mistaking the PVR and the CFAM ID. 
> >>
> >> C. 
> >>  
> > 
> > According to https://patchwork.ozlabs.org/patch/776052/
> > 
> > POWER9 DD2's PVR is expected to be 0x004e1200

Uh.. I spoke to Michael Ellerman, and he

Re: [Qemu-devel] [RFC 3/7] translate-all: use a binary search tree to track TBs in TBContext

[Richard Henderson]

On 06/29/2017 01:28 PM, Emilio G. Cota wrote:

+/* @key is already in the tree so it's safe to use container_of on it */
+static gint tc_ptr_cmp(gconstpointer candidate, gconstpointer key)
+{
+uintptr_t a = *(uintptr_t *)candidate;
+const TranslationBlock *tb = container_of(key, TranslationBlock, tc_ptr);


This I'm not keen on.  It'd be one thing if it was our own datastructure, but I 
see nothing in the GTree documentation that says that the comparison must 
always be done this way.



r~

Re: [Qemu-devel] [RFC 3/7] translate-all: use a binary search tree to track TBs in TBContext

[Richard Henderson]

On 06/30/2017 12:41 AM, Richard Henderson wrote:

On 06/29/2017 01:28 PM, Emilio G. Cota wrote:

+/* @key is already in the tree so it's safe to use container_of on it */
+static gint tc_ptr_cmp(gconstpointer candidate, gconstpointer key)
+{
+uintptr_t a = *(uintptr_t *)candidate;
+const TranslationBlock *tb = container_of(key, TranslationBlock, tc_ptr);


This I'm not keen on.  It'd be one thing if it was our own datastructure, but I 
see nothing in the GTree documentation that says that the comparison must 
always be done this way.


What if we bundle tc_ptr + tc_size into a struct and only reference that?  We'd 
embed that struct into the TB.  In tb_find_pc, create that struct on the stack, 
setting tc_size = 0.



r~

Re: [Qemu-devel] [RFC 4/7] translate-all: report correct avg host TB size

[Richard Henderson]

On 06/29/2017 01:28 PM, Emilio G. Cota wrote:

+int *size = data;


I'd really prefer you use size_t for all of these.


r~

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1

[Cédric Le Goater]

>>> According to https://patchwork.ozlabs.org/patch/776052/
>>>
>>> POWER9 DD2's PVR is expected to be 0x004e1200
> 
> Uh.. I spoke to Michael Ellerman, and he said he expected 0x004e0200.
> Though he did mention there might be several variants.  Can we please
> get a definitive answer on this from IBM.

Adding Michael Ellerman in cc: but I think Greg is correct. 

C.

Re: [Qemu-devel] [RFC 5/7] tcg: take tb_ctx out of TCGContext

[Richard Henderson]

On 06/29/2017 01:28 PM, Emilio G. Cota wrote:

Before TCGContext is made thread-local.

Signed-off-by: Emilio G. Cota
---
  include/exec/tb-context.h |  2 ++
  tcg/tcg.h |  2 --
  accel/tcg/cpu-exec.c  |  2 +-
  accel/tcg/translate-all.c | 57 ---
  linux-user/main.c |  6 ++---
  5 files changed, 35 insertions(+), 34 deletions(-)


There's a bit more of TCGContext that shouldn't be thread-local.  But I guess 
this is the one that absolutely must be shared.


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [RFC 6/7] [XXX] tcg: make TCGContext thread-local for softmmu

[Richard Henderson]

On 06/29/2017 01:28 PM, Emilio G. Cota wrote:

This will allow us to generate TCG code in parallel.

User-mode is kept out of this: contention due to concurrent translation
is more commonly found in full-system mode (e.g. booting a many-core guest).

XXX: For now, only convert arm/a64, since these are the only guests that
have proper MTTCG support.


Not true.  TCG_GUEST_DEFAULT_MO suggests we have 4.


XXX: This is calling prologue_init once per vCPU, i.e. each TCGContext
  gets a different prologue/epilogue (all of them with the same
  contents though). Far from ideal, but for an experiment it
  "should" work, right?


Um... sure.  But surely there's a better way.  Perhaps copying the main 
thread's tcg_ctx?  That'd be after prologue, and target globals are created. 
That would avoid the need to make all the target cpu_* globals be thread-local, 
for instance.


Of course, now that I think of it, this implies that my tcg patches to use 
pointers instead of indicies is not the Way Forward...



  /* Share the TCG temporaries common between 32 and 64 bit modes.  */
-extern TCGv_env cpu_env;
-extern TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
-extern TCGv_i64 cpu_exclusive_addr;
-extern TCGv_i64 cpu_exclusive_val;
+extern TCG_THREAD TCGv_env cpu_env;
+extern TCG_THREAD TCGv_i32 cpu_NF, cpu_ZF, cpu_CF, cpu_VF;
+extern TCG_THREAD TCGv_i64 cpu_exclusive_addr;
+extern TCG_THREAD TCGv_i64 cpu_exclusive_val;


It would be Really Good if we could avoid this in the end.
I see that it's the quickest way to test for now though.


@@ -887,7 +893,7 @@ typedef struct TCGOpDef {
  #endif
  } TCGOpDef;
  
-extern TCGOpDef tcg_op_defs[];

+extern TCG_THREAD TCGOpDef tcg_op_defs[];


Why?  It's constant after startup.


r~

Re: [Qemu-devel] [PATCH v1 2/3] arm: fix the armv7m reset state

[KONRAD Frederic]

On 06/29/2017 06:45 PM, Peter Maydell wrote:

On 29 June 2017 at 17:41, KONRAD Frederic  wrote:

On 06/29/2017 05:14 PM, Peter Maydell wrote:

This is awkward, because in the "we have a ROM but it's not been
copied into memory yet" case, the only thing we have is the
rom->addr, which is the address which the user's ROM blob said
it ought to be loaded in at. If the user didn't actually provide
a ROM blob that loads at 0 that seems a bit like a user error,
and I don't think this patch will catch all the cases of that
sort of mistake.



I don't think it's really a user mistake because on the real HW
the alias is configurable.. at least in my case.

There is a "jumper" setting to mirror either the Flash or the
SRAM, etc. So the binaries isn't located at 0 but at the flash
address 0x800 or some such. That's the case with u-boot and
the precompiled examples I found for this stm32f board.


  For instance if address 0 is real flash and the
high address alias is modelled by having the high address be the
alias, then if the user passes us an ELF file saying "load to
the high address" then this change won't catch that I think
(because doing the memory_region_find/get_offset_within_address_space
will return 0, which has already been tried). You'd need to
somehow have a way to say "find all the addresses within this
AS where this MR is mapped" and try them all...



This is more likely to be a user error :). Maybe we can load
the ROM before the reset but that seems a lot more invasive..


It's the same thing, though, right? If the user's ELF file
says "vector table is at 0x800" then we should either
(a) say that's a user error, or
(b) handle it right, whether we implemented the QEMU model with
the flash at 0 and the alias at 0x800, or with the flash
at 0x800 and the alias at 0.


Hi Peter,

Fondamentaly yes, it is the same.. but it seems really strange to
me to load the elf in the alias.

If I choose (a) I'll need to objcpy all the elf to 0 or modify
the build script which should work on the real board.. This seems
not really a good option to me.

If I choose (b) I won't be able to load it to SRAM and it is
basically the same result I'll need to move or modify the config.

Thanks,
Fred




BTW isn't there a trick with the ELF entry somewhere? Or is that
for the Cortex-A?


We don't honour the ELF entry point on M profile (arguably
a bug) -- armv7m_load_kernel() ignores the entry point
returned by load_elf() in the 'entry' variable.

thanks
-- PMM

Re: [Qemu-devel] [RFC 0/7] tcg: parallel code generation (Work in Progress)

[Richard Henderson]

On 06/29/2017 01:28 PM, Emilio G. Cota wrote:

- Patches 2-3 remove *tbs[] to use a binary search tree instead.
   This removes the assumption in tb_find_pc that *tbs[] are ordered
   by tc_ptr, thereby allowing us to generate code regardless of
   its location on the host (as we do after patch 6).


Have you considered a scheme by which the front end translation and tcg 
optimization are done outside the lock, but final code generation is done 
inside the lock?


It would put at least half of the translation time in the parallel space 
without requiring changes to code_buffer allocation.



r~

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1

[Laurent Vivier]

On 30/06/2017 09:12, David Gibson wrote:
> On Thu, Jun 29, 2017 at 03:42:03PM +1000, Suraj Jitindar Singh wrote:
>> On Thu, 2017-06-29 at 15:37 +1000, Suraj Jitindar Singh wrote:
>>> On Wed, 2017-06-28 at 18:41 +0200, Greg Kurz wrote:
...
>>> That makes the assumption that DD2 doesn't require any work arounds
>>> which TCG can't handle.
>>
>> Actually TCG is really a non-issue since we'll just go into the POWER9
>> architected mode.
>>
>> Can't we just have -cpu POWER9 alias to DD1 for now and add DD2 when we
>> know the pvr?
> 
> No, because calling what qemu does DD1 is simply not accurate, in
> important and guest-visible ways.
> 
> What we should do is add in DD2.0 - we know the PVR, even if the
> chip's not out yet.  Then alias POWER9 to that.
> 

OK, I have patch that define v1.0 to DD1, v2 to DD2, and set the POWER9
alias to v2, but:
- on a DD1 host and KVM HV, "-cpu host" works well and "cpu POWER9"
(that select then the v1.0) boots the kernel and hangs at early boot of
the kernel (first display)
- in TCG mode, boot by default with v2, but some services does not start
correctly, and I have never the console login (perhaps because of some
time out: I test TCG on the POWER9 host, I should test on a x86 one).

I'm trying for the moment to understand why "-cpu POWER9" hangs while
"-cpu host" works.

Thanks,
Laurent

Re: [Qemu-devel] [RFC v2 2/3] qemu-error: Implement a more generic error reporting

[Daniel P. Berrange]

On Thu, Jun 29, 2017 at 12:42:38PM -0700, Alistair Francis wrote:
> This patch removes the exisinting error_vreport() function and replaces it
> with a more generic vreport() function that takes an enum describing the
> information to be reported.
> 
> As part of this change a report() function is added as well with the
> same capability.
> 
> To maintain full compatibility the original error_report() function is
> maintained and no changes to the way errors are printed have been made.
> 
> Signed-off-by: Alistair Francis 
> ---
> 
>  hw/virtio/virtio.c  |  2 +-
>  include/qemu/error-report.h | 10 +-
>  scripts/checkpatch.pl   |  3 ++-
>  util/qemu-error.c   | 33 ++---
>  4 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 464947f76d..bd3d26abb7 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2448,7 +2448,7 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice 
> *vdev, const char *fmt, ...)
>  va_list ap;
>  
>  va_start(ap, fmt);
> -error_vreport(fmt, ap);
> +vreport(ERROR, fmt, ap);
>  va_end(ap);
>  
>  if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> index 3001865896..39b554c3b9 100644
> --- a/include/qemu/error-report.h
> +++ b/include/qemu/error-report.h
> @@ -21,6 +21,12 @@ typedef struct Location {
>  struct Location *prev;
>  } Location;
>  
> +typedef enum {
> +ERROR,
> +WARN,
> +INFO,
> +} report_types;

Woah, those are faaar to generic names to be used. There is way too much
chance of those clashing with definitions from headers we pull in - particularly
windows which pollutes its system headers with loads of generic names.

I'd suggest  QMSG_ERROR, QMSG_WARN, QMSG_INFO

> +
>  Location *loc_push_restore(Location *loc);
>  Location *loc_push_none(Location *loc);
>  Location *loc_pop(Location *loc);
> @@ -30,12 +36,14 @@ void loc_set_none(void);
>  void loc_set_cmdline(char **argv, int idx, int cnt);
>  void loc_set_file(const char *fname, int lno);
>  
> +void vreport(report_types type, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 
> 0);
> +void report(report_types type, const char *fmt, ...)  GCC_FMT_ATTR(2, 3);

Those names are too generic too IMHO.  I'd suggest  qmsg_report, qmsg_vreport

As mentioned in the previous review, there should be wrappers which
call these with suitable enum to make usage less verbose. eg

  qmsg_info(fmt, )  should call qmsg_report(QMSG_INFO, fmt, ...)
  qmsg_vinfo(fmt, )  should call qmsg_vreport(QMSG_INFO, fmt, ...)

likewise, for other message levels

>  void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
>  void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 
> 0);
>  void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  void error_set_progname(const char *argv0);
> -void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
>  void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  const char *error_get_progname(void);
>  extern bool enable_timestamp_msg;

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 v1 2/3] arm: fix the armv7m reset state

[Peter Maydell]

On 30 June 2017 at 09:24, KONRAD Frederic  wrote:
> On 06/29/2017 06:45 PM, Peter Maydell wrote:
>> It's the same thing, though, right? If the user's ELF file
>> says "vector table is at 0x800" then we should either
>> (a) say that's a user error, or
>> (b) handle it right, whether we implemented the QEMU model with
>> the flash at 0 and the alias at 0x800, or with the flash
>> at 0x800 and the alias at 0.

> Fondamentaly yes, it is the same.. but it seems really strange to
> me to load the elf in the alias.

The user creating the ELF file has no idea whether we
modelled the flash at 0 and the alias at highmem or
the other way around -- that is an implementation detail
that should be completely invisible to the user.
My two suggestions are based on that point -- either we
should treat "ELF loaded at highmem" as always wrong, or
we should always support it.

> If I choose (a) I'll need to objcpy all the elf to 0 or modify
> the build script which should work on the real board.. This seems
> not really a good option to me.

I agree that I don't like this.

> If I choose (b) I won't be able to load it to SRAM and it is
> basically the same result I'll need to move or modify the config.

I don't understand this, though. Option (b) is probably painful
to implement (I don't have a good idea of how to do it) but
it ought to mean that the ELF files that work on the board
also work for QEMU (regardless of how the board model
implemented the aliased flash).

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 3/5] virtio-9p: break device if buffers are misconfigured

[Greg Kurz]

On Fri, 30 Jun 2017 02:33:22 +0300
"Michael S. Tsirkin"  wrote:

> On Wed, Jun 28, 2017 at 10:44:30PM +0200, Greg Kurz wrote:
> > The 9P protocol is transport agnostic: if the guest misconfigured the
> > buffers, the best we can do is to set the broken flag on the device.
> > 
> > Since virtio_pdu_vmarshal() may be called by several active PDUs, we
> > check if the transport isn't broken already to avoid printing extra
> > error messages.
> > 
> > Signed-off-by: Greg Kurz   
> 
> Reviewed-by: Michael S. Tsirkin 
> 

Oops, I've already sent a pull request and it got merged.

Thanks for the Reviewed-by's anyway.

> > ---
> > v5: - use ssize_t variable in virtio_pdu_v[un]marshal()
> > - drop remaining vdev->broken check (MST suggested to discuss calling
> >   virtio_error() when the device is already broken to a separate thread)
> > ---
> >  hw/9pfs/9p.c   |2 +-
> >  hw/9pfs/9p.h   |2 +-
> >  hw/9pfs/virtio-9p-device.c |   40 
> >  hw/9pfs/xen-9p-backend.c   |3 ++-
> >  4 files changed, 40 insertions(+), 7 deletions(-)
> > 
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 96d268334865..da0d6da65b45 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> > @@ -1664,7 +1664,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector 
> > *qiov, V9fsPDU *pdu,
> >  unsigned int niov;
> >  
> >  if (is_write) {
> > -pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov);
> > +pdu->s->transport->init_out_iov_from_pdu(pdu, &iov, &niov, size + 
> > skip);
> >  } else {
> >  pdu->s->transport->init_in_iov_from_pdu(pdu, &iov, &niov, size + 
> > skip);
> >  }
> > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> > index aac1b0b2ce3d..d1cfeaf10e4f 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -363,7 +363,7 @@ struct V9fsTransport {
> >  void(*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> >  unsigned int *pniov, size_t size);
> >  void(*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > - unsigned int *pniov);
> > + unsigned int *pniov, size_t size);
> >  void(*push_and_notify)(V9fsPDU *pdu);
> >  };
> >  
> > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
> > index 1a68c1622d3a..62650b0a6b99 100644
> > --- a/hw/9pfs/virtio-9p-device.c
> > +++ b/hw/9pfs/virtio-9p-device.c
> > @@ -146,8 +146,16 @@ static ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, 
> > size_t offset,
> >  V9fsState *s = pdu->s;
> >  V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> >  VirtQueueElement *elem = v->elems[pdu->idx];
> > +ssize_t ret;
> >  
> > -return v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, 
> > ap);
> > +ret = v9fs_iov_vmarshal(elem->in_sg, elem->in_num, offset, 1, fmt, ap);
> > +if (ret < 0) {
> > +VirtIODevice *vdev = VIRTIO_DEVICE(v);
> > +
> > +virtio_error(vdev, "Failed to encode VirtFS reply type %d",
> > + pdu->id + 1);
> > +}
> > +return ret;
> >  }
> >  
> >  static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, size_t offset,
> > @@ -156,28 +164,52 @@ static ssize_t virtio_pdu_vunmarshal(V9fsPDU *pdu, 
> > size_t offset,
> >  V9fsState *s = pdu->s;
> >  V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> >  VirtQueueElement *elem = v->elems[pdu->idx];
> > +ssize_t ret;
> > +
> > +ret = v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, fmt, 
> > ap);
> > +if (ret < 0) {
> > +VirtIODevice *vdev = VIRTIO_DEVICE(v);
> >  
> > -return v9fs_iov_vunmarshal(elem->out_sg, elem->out_num, offset, 1, 
> > fmt, ap);
> > +virtio_error(vdev, "Failed to decode VirtFS request type %d", 
> > pdu->id);
> > +}
> > +return ret;
> >  }
> >  
> > -/* The size parameter is used by other transports. Do not drop it. */
> >  static void virtio_init_in_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> >  unsigned int *pniov, size_t size)
> >  {
> >  V9fsState *s = pdu->s;
> >  V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
> >  VirtQueueElement *elem = v->elems[pdu->idx];
> > +size_t buf_size = iov_size(elem->in_sg, elem->in_num);
> > +
> > +if (buf_size < size) {
> > +VirtIODevice *vdev = VIRTIO_DEVICE(v);
> > +
> > +virtio_error(vdev,
> > + "VirtFS reply type %d needs %zu bytes, buffer has 
> > %zu",
> > + pdu->id + 1, size, buf_size);
> > +}
> >  
> >  *piov = elem->in_sg;
> >  *pniov = elem->in_num;
> >  }
> >  
> >  static void virtio_init_out_iov_from_pdu(V9fsPDU *pdu, struct iovec **piov,
> > - unsigned int *pniov)
> > + unsigned in

Re: [Qemu-devel] [PATCH v6 4/6] hmp: create a throttle initialization function

[Dr. David Alan Gilbert]

* Pradeep Jagadeesh (pradeepkiruv...@gmail.com) wrote:
> This patch creates a throttle initialization function to maximize the
> code reusability. The same code is also used by fsdev.
> 
> Signed-off-by: Pradeep Jagadeesh 

Acked-by: Dr. David Alan Gilbert 

> ---
>  hmp.c | 21 +++--
>  1 file changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/hmp.c b/hmp.c
> index 8c72c58..220d301 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1749,20 +1749,29 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>  hmp_handle_error(mon, &err);
>  }
>  
> +static void hmp_initialize_io_throttle(IOThrottle *iot, const QDict *qdict)
> +{
> +iot->has_id = true;
> +iot->id = (char *) qdict_get_str(qdict, "id");
> +iot->bps = qdict_get_int(qdict, "bps");
> +iot->bps_rd = qdict_get_int(qdict, "bps_rd");
> +iot->bps_wr = qdict_get_int(qdict, "bps_wr");
> +iot->iops = qdict_get_int(qdict, "iops");
> +iot->iops_rd = qdict_get_int(qdict, "iops_rd");
> +iot->iops_wr = qdict_get_int(qdict, "iops_wr");
> +}
> +
>  void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
>  {
>  Error *err = NULL;
> +IOThrottle *iothrottle;
>  BlockIOThrottle throttle = {
>  .has_device = true,
>  .device = (char *) qdict_get_str(qdict, "device"),
> -.bps = qdict_get_int(qdict, "bps"),
> -.bps_rd = qdict_get_int(qdict, "bps_rd"),
> -.bps_wr = qdict_get_int(qdict, "bps_wr"),
> -.iops = qdict_get_int(qdict, "iops"),
> -.iops_rd = qdict_get_int(qdict, "iops_rd"),
> -.iops_wr = qdict_get_int(qdict, "iops_wr"),
>  };
>  
> +iothrottle = qapi_BlockIOThrottle_base(&throttle);
> +hmp_initialize_io_throttle(iothrottle, qdict);
>  qmp_block_set_io_throttle(&throttle, &err);
>  hmp_handle_error(mon, &err);
>  }
> -- 
> 1.8.3.1
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC PATCH 0/8] removal of tci (tcg interpreter)

[Peter Maydell]

On 29 June 2017 at 19:09, Stefan Weil  wrote:
> Nevertheless QEMU is the only software which I know which not
> only addresses the needs for server / PC virtualisation, but
> also is useful for researchers and scientists, archivists,
> learners and teachers or experimenters.
>
> Up to now, I had the impression that QEMU is sufficiently modular
> so that the different perspectives and expectations could be
> supported without harming each other too much. The only drawback
> was additional code (which would not be needed when focussing on
> a single perspective) causing additional compilation time
> and more (formal, mostly automated) file changes for code clean
> ups and API‌ changes.

Yes, the whole of TCG demonstrates that we're not purely
trying to be a virtual-machine-usecase-only codebase,
so certainly there is scope for having multiple parts
and uses in QEMU.

I think the part of TCI that makes it stand out as awkward
here is that it is an option that you have to pick at
configure time, and enabling it disables the standard
TCG backend. If it was a runtime option to use the
interpreter rather than the codegen (in the same way that
you can runtime select between KVM and TCG), I think that
would help a lot:
 * reduces the number of build configs we need to test
 * means it's accessible to the bulk of users who use their
   distro's build of QEMU rather than rolling their own
 * less of a beartrap for users who enable the config
   option and then wonder why QEMU is running so slowly
 * opens the possibility of using the interpreter as part
   of mainstream TCG (eg in the cases of "execute one
   insn and throw it away, which might be faster
   interpreted in theory)

TCI is also awkward because it means we don't really know
what the set of platforms we run on is. (For instance I'd
like to fix our configure script which currently uses 'uname'
as a fallback for identifying target CPU and OS, which
is totally wrong for cross-compile. It would be easier to
be sure such a refactor was correct if we knew which
hosts we could possibly run on...)

That to me all adds up to a position where while I don't
care enough to take active steps to remove TCI from QEMU,
I wouldn't be unhappy to see it go if other people strongly
support removal, and if nobody's interested in trying to
fix its problems.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v6 5/6] fsdev: hmp interface for throttling

[Dr. David Alan Gilbert]

* Pradeep Jagadeesh (pradeepkiruv...@gmail.com) wrote:
> This patch introduces hmp interfaces for the fsdev
> devices.
> 
> Signed-off-by: Pradeep Jagadeesh 
> ---
>  hmp-commands-info.hx | 18 ++
>  hmp-commands.hx  | 19 +++
>  hmp.c| 66 
> 
>  hmp.h|  4 
>  4 files changed, 107 insertions(+)
> 
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index ae16901..f23b627 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -84,6 +84,24 @@ STEXI
>  Show block device statistics.
>  ETEXI
>  
> +#if defined(CONFIG_VIRTFS)
> +
> +{
> +.name   = "query-fsdev-iothrottle",

This will end up as:
info query-fsdev-iothrottle

so the query- is unneeded since it's already info.

> +.args_type  = "",
> +.params = "",
> +.help   = "show fsdev device throttle information",
> +.cmd= hmp_fsdev_get_io_throttle,
> +},
> +
> +#endif
> +
> +STEXI
> +@item info fsdev throttle

I think that's supposed to match the .name - i.e.
   @item info query-fsdev-iothrottle
 
 (or whatever it will become)

> +@findex fsdevthrottleinfo

again I think that's supposed to match the .name - see the other entries
in that file.

> +Show fsdev device throttleinfo.

And we may as well make that text match the .help text.

> +ETEXI
> +
>  {
>  .name   = "block-jobs",
>  .args_type  = "",
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e763606..c60fd7e 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1662,6 +1662,25 @@ STEXI
>  Change I/O throttle limits for a block drive to @var{bps} @var{bps_rd} 
> @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
>  ETEXI
>  
> +#if defined(CONFIG_VIRTFS)
> +
> +{
> +.name   = "fsdev-set-io-throttle",
> +.args_type  = 
> "device:B,bps:l,bps_rd:l,bps_wr:l,iops:l,iops_rd:l,iops_wr:l",
> +.params = "device bps bps_rd bps_wr iops iops_rd iops_wr",
> +.help   = "change I/O throttle limits for a fs devices",
> +.cmd= hmp_fsdev_set_io_throttle,
> +},
> +
> +#endif
> +
> +STEXI
> +@item fsdev_set_io_throttle @var{device} @var{bps} @var{bps_rd} @var{bps_wr} 
> @var{iops} @var{iops_rd} @var{iops_wr}
> +@findex fsdev_set_io_throttle

Again I think the @item and @findex have to match the .name - so I think
make the .name use _'s rather than -'s for HMP.

> +Change I/O throttle limits for a fs devices to @var{bps} @var{bps_rd} 
> @var{bps_wr} @var{iops} @var{iops_rd} @var{iops_wr}
> +ETEXI
> +
> +
>  {
>  .name   = "set_password",
>  .args_type  = "protocol:s,password:s,connected:s?",
> diff --git a/hmp.c b/hmp.c
> index 220d301..b1c698b 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1776,6 +1776,72 @@ void hmp_block_set_io_throttle(Monitor *mon, const 
> QDict *qdict)
>  hmp_handle_error(mon, &err);
>  }
>  
> +#ifdef CONFIG_VIRTFS
> +
> +void hmp_fsdev_set_io_throttle(Monitor *mon, const QDict *qdict)
> +{
> +Error *err = NULL;
> +IOThrottle throttle;
> +
> +hmp_initialize_io_throttle(&throttle, qdict);
> +qmp_fsdev_set_io_throttle(&throttle, &err);
> +hmp_handle_error(mon, &err);
> +}
> +
> +static void print_fsdev_throttle_config(Monitor *mon, IOThrottle *fscfg,
> +   Error *err)
> +{
> +if (fscfg->bps  || fscfg->bps_rd  || fscfg->bps_wr  ||
> +fscfg->iops || fscfg->iops_rd || fscfg->iops_wr)
> +{
> +monitor_printf(mon, "%s", fscfg->id);
> +monitor_printf(mon, "I/O throttling:"
> +" bps=%" PRId64
> +" bps_rd=%" PRId64  " bps_wr=%" PRId64
> +" bps_max=%" PRId64
> +" bps_rd_max=%" PRId64
> +" bps_wr_max=%" PRId64
> +" iops=%" PRId64 " iops_rd=%" PRId64
> +" iops_wr=%" PRId64
> +" iops_max=%" PRId64
> +" iops_rd_max=%" PRId64
> +" iops_wr_max=%" PRId64
> +" iops_size=%" PRId64,
> +fscfg->bps,
> +fscfg->bps_rd,
> +fscfg->bps_wr,
> +fscfg->bps_max,
> +fscfg->bps_rd_max,
> +fscfg->bps_wr_max,
> +fscfg->iops,
> +fscfg->iops_rd,
> +fscfg->iops_wr,
> +fscfg->iops_max,
> +fscfg->iops_rd_max,
> +fscfg->iops_wr_max,
> +fscfg->iops_size);
> +   }
> +   hmp_handle_error(mon, &err);

You've not got anything that can generate an error here have you?

> +}
> +
> +void hmp_fsdev_get_io_throttle(Monitor *mon, const QDict *qdict)
> +{
> +Error *

[Qemu-devel] [PATCH] spapr: fix bogus function name in comment

[Greg Kurz]

$ git grep spapr_ppc_reset
hw/ppc/spapr.c: * as part of spapr_ppc_reset().

$ git grep ppc_spapr_reset
hw/ppc/spapr.c:static void ppc_spapr_reset(void)
hw/ppc/spapr.c:mc->reset = ppc_spapr_reset;
hw/ppc/spapr_hcall.c:/* If ppc_spapr_reset() did not set up a HPT
 but one is necessary

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0ee9fac50bd4..43a1cb5725d6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1973,7 +1973,7 @@ static void spapr_boot_set(void *opaque, const char 
*boot_device,
  * Unlike PCI DR devices, LMB DR devices explicitly register this reset
  * routine. Reset for PCI DR devices will be handled by PHB reset routine
  * when it walks all its children devices. LMB devices reset occurs
- * as part of spapr_ppc_reset().
+ * as part of ppc_spapr_reset().
  */
 static void spapr_drc_reset(void *opaque)
 {

Re: [Qemu-devel] [PATCH] tests: Avoid non-portable 'echo -ARG'

[Stefan Hajnoczi]

On Wed, Jun 28, 2017 at 09:21:37AM -0500, Eric Blake wrote:
> POSIX says that backslashes in the arguments to 'echo', as well as
> any use of 'echo -n' and 'echo -e', are non-portable; it recommends
> people should favor 'printf' instead.  This is definitely true where
> we do not control which shell is running (such as in makefile snippets
> or in documentation examples).  But even for scripts where we
> require bash (and therefore, where echo does what we want by default),
> it is still possible to use 'shopt -s xpg_echo' to change bash's
> behavior of echo.  And setting a good example never hurts when we are
> not sure if a snippet will be copied from a bash-only script to a
> general shell script (although I don't change the use of non-portable
> \e for ESC when we know the running shell is bash).
> 
> Replace 'echo -n "..."' with 'printf "..."', and 'echo -e "..."'
> with 'printf "...\n"'.
> 
> In the qemu-iotests check script, also fix unusual shell quoting
> that would result in word-splitting if 'date' outputs a space.
> 
> Signed-off-by: Eric Blake 
> ---
> 
> Of course, Stefan's pending patch:
> [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
> also touches 068, so there may be some (obvious) merge conflicts
> to resolve there depending on what goes in first.
> 
>  qemu-options.hx |  4 ++--
>  tests/multiboot/run_test.sh | 10 +-
>  tests/qemu-iotests/051  |  7 ---
>  tests/qemu-iotests/068  |  2 +-
>  tests/qemu-iotests/142  | 48 
> ++---
>  tests/qemu-iotests/171  | 14 ++---
>  tests/qemu-iotests/check| 18 -
>  tests/rocker/all| 10 +-
>  tests/tcg/cris/Makefile |  8 
>  9 files changed, 61 insertions(+), 60 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 2/3] util/oslib-win32: Remove invalid check

[Paolo Bonzini]

On 29/06/2017 19:16, Alistair Francis wrote:
> There is no way nhandles can be zero in this section so that part of the
> if statement will always be false. Let's just remove it to make the code
> easier to read.
> 
> Signed-off-by: Alistair Francis 
> Acked-by: Edgar E. Iglesias 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Fam Zheng 
> ---
> 
>  util/oslib-win32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 80e4668935..7ec0f8e083 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -414,7 +414,7 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles, 
> gint nhandles,
>  /* If we have a timeout, or no handles to poll, be satisfied
>   * with just noticing we have messages waiting.
>   */
> -if (timeout != 0 || nhandles == 0) {
> +if (timeout != 0) {
>  return 1;
>  }
>  

Can you explain this better?

Paolo

[Qemu-devel] [PATCH] spapr: refresh "platform-specific" hcalls comment

[Greg Kurz]

We have more of these since the addition of KVMPPC_H_LOGICAL_MEMOP in 2012.

Signed-off-by: Greg Kurz 
---
 include/hw/ppc/spapr.h |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index a66bbac35242..1826cc4fd696 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -377,9 +377,8 @@ struct sPAPRMachineState {
  * as well.
  *
  * We also need some hcalls which are specific to qemu / KVM-on-POWER.
- * So far we just need one for H_RTAS, but in future we'll need more
- * for extensions like virtio.  We put those into the 0xf000-0xfffc
- * range which is reserved by PAPR for "platform-specific" hcalls.
+ * We put those into the 0xf000-0xfffc range which is reserved by PAPR
+ * for "platform-specific" hcalls.
  */
 #define KVMPPC_HCALL_BASE   0xf000
 #define KVMPPC_H_RTAS   (KVMPPC_HCALL_BASE + 0x0)

Re: [Qemu-devel] [PATCH v1 2/3] util/oslib-win32: Remove invalid check

[Peter Maydell]

On 29 June 2017 at 18:16, Alistair Francis  wrote:
> There is no way nhandles can be zero in this section so that part of the
> if statement will always be false. Let's just remove it to make the code
> easier to read.
>
> Signed-off-by: Alistair Francis 
> Acked-by: Edgar E. Iglesias 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Fam Zheng 
> ---
>
>  util/oslib-win32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 80e4668935..7ec0f8e083 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -414,7 +414,7 @@ static int poll_rest(gboolean poll_msgs, HANDLE *handles, 
> gint nhandles,
>  /* If we have a timeout, or no handles to poll, be satisfied
>   * with just noticing we have messages waiting.
>   */
> -if (timeout != 0 || nhandles == 0) {
> +if (timeout != 0) {
>  return 1;
>  }

The comment and the code now don't match -- does the comment
need updating too?

thanks
-- PMM

[Qemu-devel] Running QEMU for Microblaze

[Ormaetxea Xabier]

Hello!

I'm trying to run QEMU-Microblaze (Little-endian) with a standalone app in some 
different ways, but none of them works for me:


1)  I have created my own .DTB from my system design (.HDF), just a 
microblaze connected to the Uartlite AXI, leds, interrupt controller, and a 
gpio. Using the board support package I've made a simple c-based program (at 
the end of the message):

Run it with ./qemu-system-microblazeel -M microblaze-fdt-plnx -dtb 
system-top.dtb -kernel fibonacci.elf (-s -S) #(-s -S just to debug it)

Invalid MicroBlaze version number: (null)   #Don't think it's a problem
Bar ram offset 9000528f
Aborted (core dumped)


2)  I use a mb.dtb that I found on the internet. Same c-based program. Run 
it with the debugger:

./qemu-system-microblazeel -M microblaze-fdt-plnx -dtb mb.dtb -kernel 
fibonacci.elf (-s -S)

Lets my debug it (doesn't fail booting), but in the first step:
Bad ram pointer

I suppose this isn't my option cause the .dtb is created for another 
microblaze-based design.


3)  I use the same design (.HDF), but run it for the Spartan 3a dsp 1800 
option:

./qemu-system-microblazeel -kernel fibonacci.elf (-s -S)

Runs "properly" -> I mean properly because I can follow on the debugger that 
the steps are correctly made:
_start -> _start1 -> main -> fibonacci -> xil_printf -> xil_printf ... -> 
xil_printf -> _exit

But doesn't print nothing. And doesn't write in memory as asked (*addrPtr = 
0x150) :
In the qemu shell:
(qemu) x 0xC000
C000: 0x#When it should be 0x0150


4)  Modifying my system design to be similar to the Spartan board design:
MEMORY_BASEADDR 0x9000
FLASH_BASEADDR 0xa000
INTC_BASEADDR 0x8180
TIMER_BASEADDR 0x83c0
UARTLITE_BASEADDR 0x8400
ETHLITE_BASEADDR 0x8100
I get exactly the same result as in the (3) case.

So here they go my questions:

-  Am I doing it right? Is this the method of running a standalone 
program over a microblaze?

-  How can I make the program print something?

-  Im not sure if the problem is that it doesn't write on memory, or I 
am the one who fails reading it from the shell, because if I change the writing 
position to (0x8408, uart status position) I get the error (qemu: hardware 
error: write to UART STATUS?) -> that means im writing (or trying, at least). 
How can I write on memory (and read after it, so I know it works)?

Thank you in advance! Really appreciate your Job! (I'm sorry if my problem it's 
a simple one, I'm new at it)

#include 
#include "platform.h"
#include "xil_printf.h"

void fibonacci(int d){
   u32 a=1;
   u32 b=0;
   u32 em=0;
   u32 *addrPtr = 0xC000;

for(int num=0; num

[Qemu-devel] [PATCH v7 2/3] migration: introduce qemu_ufd_copy_ioctl helper

[Alexey Perevalov]

Just for placing auxilary operations inside helper,
auxilary operations like: track received pages,
notify about copying operation in futher patches.

Reviewed-by: Juan Quintela 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Peter Xu 
Signed-off-by: Alexey Perevalov 
---
 migration/postcopy-ram.c | 34 +-
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 996e64d..be497bb 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -559,6 +559,25 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
 return 0;
 }
 
+static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
+void *from_addr, uint64_t pagesize)
+{
+if (from_addr) {
+struct uffdio_copy copy_struct;
+copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
+copy_struct.src = (uint64_t)(uintptr_t)from_addr;
+copy_struct.len = pagesize;
+copy_struct.mode = 0;
+return ioctl(userfault_fd, UFFDIO_COPY, ©_struct);
+} else {
+struct uffdio_zeropage zero_struct;
+zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
+zero_struct.range.len = pagesize;
+zero_struct.mode = 0;
+return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
+}
+}
+
 /*
  * Place a host page (from) at (host) atomically
  * returns 0 on success
@@ -566,20 +585,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState 
*mis)
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
 RAMBlock *rb)
 {
-struct uffdio_copy copy_struct;
 size_t pagesize = qemu_ram_pagesize(rb);
 
-copy_struct.dst = (uint64_t)(uintptr_t)host;
-copy_struct.src = (uint64_t)(uintptr_t)from;
-copy_struct.len = pagesize;
-copy_struct.mode = 0;
-
 /* copy also acks to the kernel waking the stalled thread up
  * TODO: We can inhibit that ack and only do it if it was requested
  * which would be slightly cheaper, but we'd have to be careful
  * of the order of updating our page state.
  */
-if (ioctl(mis->userfault_fd, UFFDIO_COPY, ©_struct)) {
+if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
 int e = errno;
 error_report("%s: %s copy host: %p from: %p (size: %zd)",
  __func__, strerror(e), host, from, pagesize);
@@ -601,12 +614,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, 
void *host,
 trace_postcopy_place_page_zero(host);
 
 if (qemu_ram_pagesize(rb) == getpagesize()) {
-struct uffdio_zeropage zero_struct;
-zero_struct.range.start = (uint64_t)(uintptr_t)host;
-zero_struct.range.len = getpagesize();
-zero_struct.mode = 0;
-
-if (ioctl(mis->userfault_fd, UFFDIO_ZEROPAGE, &zero_struct)) {
+if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, getpagesize())) 
{
 int e = errno;
 error_report("%s: %s zero host: %p",
  __func__, strerror(e), host);
-- 
1.8.3.1

[Qemu-devel] [PATCH v7 1/3] migration: postcopy_place_page factoring out

[Alexey Perevalov]

Need to mark copied pages as closer as possible to the place where it
tracks down. That will be necessary in futher patch.

Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Peter Xu 
Reviewed-by: Juan Quintela 
Signed-off-by: Alexey Perevalov 
---
 migration/postcopy-ram.c | 13 +++--
 migration/postcopy-ram.h |  4 ++--
 migration/ram.c  |  4 ++--
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 7e21e6f..996e64d 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -564,9 +564,10 @@ int postcopy_ram_enable_notify(MigrationIncomingState *mis)
  * returns 0 on success
  */
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
-size_t pagesize)
+RAMBlock *rb)
 {
 struct uffdio_copy copy_struct;
+size_t pagesize = qemu_ram_pagesize(rb);
 
 copy_struct.dst = (uint64_t)(uintptr_t)host;
 copy_struct.src = (uint64_t)(uintptr_t)from;
@@ -595,11 +596,11 @@ int postcopy_place_page(MigrationIncomingState *mis, void 
*host, void *from,
  * returns 0 on success
  */
 int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
- size_t pagesize)
+ RAMBlock *rb)
 {
 trace_postcopy_place_page_zero(host);
 
-if (pagesize == getpagesize()) {
+if (qemu_ram_pagesize(rb) == getpagesize()) {
 struct uffdio_zeropage zero_struct;
 zero_struct.range.start = (uint64_t)(uintptr_t)host;
 zero_struct.range.len = getpagesize();
@@ -629,7 +630,7 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, 
void *host,
 memset(mis->postcopy_tmp_zero_page, '\0', mis->largest_page_size);
 }
 return postcopy_place_page(mis, host, mis->postcopy_tmp_zero_page,
-   pagesize);
+   rb);
 }
 
 return 0;
@@ -692,14 +693,14 @@ int postcopy_ram_enable_notify(MigrationIncomingState 
*mis)
 }
 
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
-size_t pagesize)
+RAMBlock *rb)
 {
 assert(0);
 return -1;
 }
 
 int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
-size_t pagesize)
+RAMBlock *rb)
 {
 assert(0);
 return -1;
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index 52d51e8..78a3591 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -72,14 +72,14 @@ void postcopy_discard_send_finish(MigrationState *ms,
  * returns 0 on success
  */
 int postcopy_place_page(MigrationIncomingState *mis, void *host, void *from,
-size_t pagesize);
+RAMBlock *rb);
 
 /*
  * Place a zero page at (host) atomically
  * returns 0 on success
  */
 int postcopy_place_page_zero(MigrationIncomingState *mis, void *host,
- size_t pagesize);
+ RAMBlock *rb);
 
 /* The current postcopy state is read/set by postcopy_state_get/set
  * which update it atomically.
diff --git a/migration/ram.c b/migration/ram.c
index 1b08296..9cc1b17 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2470,10 +2470,10 @@ static int ram_load_postcopy(QEMUFile *f)
 
 if (all_zero) {
 ret = postcopy_place_page_zero(mis, place_dest,
-   block->page_size);
+   block);
 } else {
 ret = postcopy_place_page(mis, place_dest,
-  place_source, block->page_size);
+  place_source, block);
 }
 }
 if (!ret) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v7 3/3] migration: add bitmap for received page

[Alexey Perevalov]

This patch adds ability to track down already received
pages, it's necessary for calculation vCPU block time in
postcopy migration feature, maybe for restore after
postcopy migration failure.
Also it's necessary to solve shared memory issue in
postcopy livemigration. Information about received pages
will be transferred to the software virtual bridge
(e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
already received pages. fallocate syscall is required for
remmaped shared memory, due to remmaping itself blocks
ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
error (struct page is exists after remmap).

Bitmap is placed into RAMBlock as another postcopy/precopy
related bitmaps.

Signed-off-by: Alexey Perevalov 
---
 include/exec/ram_addr.h  | 10 ++
 migration/postcopy-ram.c | 16 +++-
 migration/ram.c  | 43 ---
 migration/ram.h  |  6 ++
 4 files changed, 67 insertions(+), 8 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index 73d1bea..af5bf26 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -47,6 +47,8 @@ struct RAMBlock {
  * of the postcopy phase
  */
 unsigned long *unsentmap;
+/* bitmap of already received pages in postcopy */
+unsigned long *receivedmap;
 };
 
 static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
@@ -60,6 +62,14 @@ static inline void *ramblock_ptr(RAMBlock *block, ram_addr_t 
offset)
 return (char *)block->host + offset;
 }
 
+static inline unsigned long int ramblock_recv_bitmap_offset(void *host_addr,
+RAMBlock *rb)
+{
+uint64_t host_addr_offset =
+(uint64_t)(uintptr_t)(host_addr - (void *)rb->host);
+return host_addr_offset >> TARGET_PAGE_BITS;
+}
+
 long qemu_getrampagesize(void);
 unsigned long last_ram_page(void);
 RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index be497bb..276ce12 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -560,22 +560,27 @@ int postcopy_ram_enable_notify(MigrationIncomingState 
*mis)
 }
 
 static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
-void *from_addr, uint64_t pagesize)
+   void *from_addr, uint64_t pagesize, RAMBlock 
*rb)
 {
+int ret;
 if (from_addr) {
 struct uffdio_copy copy_struct;
 copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
 copy_struct.src = (uint64_t)(uintptr_t)from_addr;
 copy_struct.len = pagesize;
 copy_struct.mode = 0;
-return ioctl(userfault_fd, UFFDIO_COPY, ©_struct);
+ret = ioctl(userfault_fd, UFFDIO_COPY, ©_struct);
 } else {
 struct uffdio_zeropage zero_struct;
 zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
 zero_struct.range.len = pagesize;
 zero_struct.mode = 0;
-return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
+ret = ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
+}
+if (!ret) {
+ramblock_recv_bitmap_set(host_addr, rb);
 }
+return ret;
 }
 
 /*
@@ -592,7 +597,7 @@ int postcopy_place_page(MigrationIncomingState *mis, void 
*host, void *from,
  * which would be slightly cheaper, but we'd have to be careful
  * of the order of updating our page state.
  */
-if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
+if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {
 int e = errno;
 error_report("%s: %s copy host: %p from: %p (size: %zd)",
  __func__, strerror(e), host, from, pagesize);
@@ -614,7 +619,8 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, 
void *host,
 trace_postcopy_place_page_zero(host);
 
 if (qemu_ram_pagesize(rb) == getpagesize()) {
-if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, getpagesize())) 
{
+if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, getpagesize(),
+rb)) {
 int e = errno;
 error_report("%s: %s zero host: %p",
  __func__, strerror(e), host);
diff --git a/migration/ram.c b/migration/ram.c
index 9cc1b17..dfbb36b 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -147,6 +147,32 @@ out:
 return ret;
 }
 
+void ramblock_recv_map_init(void)
+{
+RAMBlock *rb;
+
+RAMBLOCK_FOREACH(rb) {
+assert(!rb->receivedmap);
+rb->receivedmap = bitmap_new(rb->max_length >> TARGET_PAGE_BITS);
+}
+}
+
+int ramblock_recv_bitmap_test(void *host_addr, RAMBlock *rb)
+{
+return test_bit(ramblock_recv_bitmap_offset(host_addr, rb),
+rb->receivedmap);
+}
+
+void ramblock_recv_bitmap_set(void *host_addr, RAMBlock *rb)
+{
+set_bit_atomic(ramblock_recv_bitmap_offset(host_addr,

Re: [Qemu-devel] [PATCH 0/6] virtio: use ioeventfd in TCG and qtest mode

[Stefan Hajnoczi]

On Wed, Jun 28, 2017 at 07:47:18PM +0100, Stefan Hajnoczi wrote:
> This patch series fixes qemu-iotests 068.  Since commit
> ea4f3cebc4e0224605ab9dd9724aa4e7768fe372 ("qemu-iotests: 068: test iothread
> mode") the test case has attempted to use dataplane without -M accel=kvm.
> Although QEMU is capable of running TCG or qtest with emulated ioeventfd/irqfd
> we haven't enabled it yet.
> 
> Unfortunately the virtio test cases fail when ioeventfd is enabled in qtest
> mode.  This is because they make assumptions about virtqueue ISR signalling.
> They assume that a request is completed when ISR becomes 1.  However, the ISR
> can be set to 1 even though no new request has completed since commit
> 83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
> notifications".
> 
> This issue is solved by introducing a proper qvirtqueue_get_buf() API (similar
> to the Linux guest drivers) instead of making assumptions about the ISR.  Most
> of the patches update the test cases to use the new API.
> 
> Stefan Hajnoczi (6):
>   libqos: fix typo in virtio.h QVirtQueue->used comment
>   libqos: add virtio used ring support
>   tests: fix virtio-scsi-test ISR dependence
>   tests: fix virtio-blk-test ISR dependence
>   tests: fix virtio-net-test ISR dependence
>   virtio-pci: use ioeventfd even when KVM is disabled
> 
>  tests/libqos/virtio.h|  8 ++-
>  hw/virtio/virtio-pci.c   |  2 +-
>  tests/libqos/virtio.c| 60 
> 
>  tests/virtio-blk-test.c  | 27 ++
>  tests/virtio-net-test.c  |  6 ++---
>  tests/virtio-scsi-test.c |  2 +-
>  6 files changed, 89 insertions(+), 16 deletions(-)
> 
> -- 
> 2.9.4
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH] spapr: refresh "platform-specific" hcalls comment

[Thomas Huth]

On 30.06.2017 12:05, Greg Kurz wrote:
> We have more of these since the addition of KVMPPC_H_LOGICAL_MEMOP in 2012.

Right, and we don't need one for virtio :-)

> Signed-off-by: Greg Kurz 
> ---
>  include/hw/ppc/spapr.h |5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index a66bbac35242..1826cc4fd696 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -377,9 +377,8 @@ struct sPAPRMachineState {
>   * as well.
>   *
>   * We also need some hcalls which are specific to qemu / KVM-on-POWER.
> - * So far we just need one for H_RTAS, but in future we'll need more
> - * for extensions like virtio.  We put those into the 0xf000-0xfffc
> - * range which is reserved by PAPR for "platform-specific" hcalls.
> + * We put those into the 0xf000-0xfffc range which is reserved by PAPR
> + * for "platform-specific" hcalls.
>   */
>  #define KVMPPC_HCALL_BASE   0xf000
>  #define KVMPPC_H_RTAS   (KVMPPC_HCALL_BASE + 0x0)

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target/ppc/cpu-models: set POWER9_v1.0 as POWER9 DD1

[Michael Ellerman]

Cédric Le Goater  writes:

 According to https://patchwork.ozlabs.org/patch/776052/

 POWER9 DD2's PVR is expected to be 0x004e1200
>> 
>> Uh.. I spoke to Michael Ellerman, and he said he expected 0x004e0200.
>> Though he did mention there might be several variants.  Can we please
>> get a definitive answer on this from IBM.
>
> Adding Michael Ellerman in cc: but I think Greg is correct. 

I don't claim to be correct :)

AFAIK Mikey's patch (linked above) is the most definitive public
documentation we have on this.

Which says:
  The P9 PVR bits 12-15 don't indicate a revision but instead different
  chip configurations.  From BookIV we have:
 Bits  Configuration
  0 :Scale out 12 cores
  1 :Scale out 24 cores
  2 :Scale up  12 cores
  3 :Scale up  24 cores

His heading of "Bits" is somewhat confusing, I'm pretty sure he just
means "the decimal value of bits 12-15", not a bit number.

Which means there may be "POWER9 DD2" chips with PVRs of:
  - 0x004e02xx
  - 0x004e12xx
  - 0x004e22xx
  - 0x004e32xx
 
So the question is just which of the scale out values to use. Neither
is technically correct, because Qemu won't necessarily be emulating a 12
or 24 core system.

Mikey did say "Linux will mostly use the "Scale out 24 core"
configuration", but AFAIK that's really just about the system designs
that are currently in plan.

Hope that makes it clear as mud :)

cheers

Re: [Qemu-devel] [RFC v1 2/4] util/oslib-win32: Remove invalid check

[Paolo Bonzini]

On 29/06/2017 18:37, Alistair Francis wrote:
>> Hmm, I think it's possible, poll_msgs is true here.
> poll_msgs?
> 
> If nhandles is 0 then we have already entered an earlier if statement
> and set ready to either WAIT_FAILED or WAIT_TIMEOUT in which case we
> can't enter this part of the if statement.

No, that's not correct.  The code is:

if (poll_msgs) {
/* Wait for either messages or handles
 * -> Use MsgWaitForMultipleObjectsEx
 */
ready = MsgWaitForMultipleObjectsEx(nhandles, handles, timeout,
QS_ALLINPUT, MWMO_ALERTABLE);

if (ready == WAIT_FAILED) {
gchar *emsg = g_win32_error_message(GetLastError());
g_warning("MsgWaitForMultipleObjectsEx failed: %s", emsg);
g_free(emsg);
}
} else if (nhandles == 0) {
/* No handles to wait for, just the timeout */
if (timeout == INFINITE) {
ready = WAIT_FAILED;
} else {
SleepEx(timeout, TRUE);
ready = WAIT_TIMEOUT;
}

You can have poll_msgs == TRUE && nhandles == 0.  This happens for

   GPollFD fds[1] = { .fd = G_WIN32_MSG_HANDLE, .events = G_IO_IN };
   g_poll(fds, 1, timeout);

Thanks,

Paolo

Re: [Qemu-devel] [PATCH] qom: enforce readonly nature of link's check callback

[Paolo Bonzini]

On 29/06/2017 13:14, Igor Mammedov wrote:
> link's check callback is supposed to verify/permit setting it,
> however currently nothing restricts it from misusing it
> and modifying target object from within.
> Make sure that readonly semantics are checked by compiler
> to prevent callback's misuse.
> 
> Signed-off-by: Igor Mammedov 
> ---
> Fam,
>  it probably conflicts with yours DEFINE_PROP_LINK series,
>  feel free to include this patch if you'll have to respin

Since there will be a respin, I'm _not_ queuing this patch.

Paolo

> ---
>  include/hw/qdev-properties.h | 3 ++-
>  include/qom/object.h | 6 +++---
>  hw/core/qdev-properties.c| 3 ++-
>  hw/display/xlnx_dp.c | 2 +-
>  hw/ipmi/ipmi.c   | 2 +-
>  hw/mem/pc-dimm.c | 2 +-
>  hw/misc/ivshmem.c| 2 +-
>  qom/object.c | 8 
>  8 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index 306bbab..6dfe16e 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -234,7 +234,8 @@ void qdev_prop_set_after_realize(DeviceState *dev, const 
> char *name,
>   * This function should be used as the check() argument to
>   * object_property_add_link().
>   */
> -void qdev_prop_allow_set_link_before_realize(Object *obj, const char *name,
> +void qdev_prop_allow_set_link_before_realize(const Object *obj,
> + const char *name,
>   Object *val, Error **errp);
>  
>  #endif
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 5ecc2d1..5223692 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -788,7 +788,7 @@ ObjectClass *object_get_class(Object *obj);
>   *
>   * Returns: The QOM typename of @obj.
>   */
> -const char *object_get_typename(Object *obj);
> +const char *object_get_typename(const Object *obj);
>  
>  /**
>   * type_register_static:
> @@ -1320,7 +1320,7 @@ typedef enum {
>   * callback function.  It allows the link property to be set and never 
> returns
>   * an error.
>   */
> -void object_property_allow_set_link(Object *, const char *,
> +void object_property_allow_set_link(const Object *, const char *,
>  Object *, Error **);
>  
>  /**
> @@ -1353,7 +1353,7 @@ void object_property_allow_set_link(Object *, const 
> char *,
>   */
>  void object_property_add_link(Object *obj, const char *name,
>const char *type, Object **child,
> -  void (*check)(Object *obj, const char *name,
> +  void (*check)(const Object *obj, const char 
> *name,
>  Object *val, Error **errp),
>ObjectPropertyLinkFlags flags,
>Error **errp);
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 2a82768..95e5fdb 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -25,7 +25,8 @@ void qdev_prop_set_after_realize(DeviceState *dev, const 
> char *name,
>  }
>  }
>  
> -void qdev_prop_allow_set_link_before_realize(Object *obj, const char *name,
> +void qdev_prop_allow_set_link_before_realize(const Object *obj,
> + const char *name,
>   Object *val, Error **errp)
>  {
>  DeviceState *dev = DEVICE(obj);
> diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
> index f43eb09..3ed81ff 100644
> --- a/hw/display/xlnx_dp.c
> +++ b/hw/display/xlnx_dp.c
> @@ -515,7 +515,7 @@ static void xlnx_dp_aux_set_command(XlnxDPState *s, 
> uint32_t value)
>  s->core_registers[DP_INTERRUPT_SIGNAL_STATE] |= 0x04;
>  }
>  
> -static void xlnx_dp_set_dpdma(Object *obj, const char *name, Object *val,
> +static void xlnx_dp_set_dpdma(const Object *obj, const char *name, Object 
> *val,
>Error **errp)
>  {
>  XlnxDPState *s = XLNX_DP(obj);
> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
> index 5cf1caa..a2fd1eb 100644
> --- a/hw/ipmi/ipmi.c
> +++ b/hw/ipmi/ipmi.c
> @@ -90,7 +90,7 @@ static TypeInfo ipmi_interface_type_info = {
>  .class_init = ipmi_interface_class_init,
>  };
>  
> -static void isa_ipmi_bmc_check(Object *obj, const char *name,
> +static void isa_ipmi_bmc_check(const Object *obj, const char *name,
> Object *val, Error **errp)
>  {
>  IPMIBmc *bmc = IPMI_BMC(val);
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 9e8dab0..380cb30 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -366,7 +366,7 @@ static void pc_dimm_get_size(Object *obj, Visitor *v, 
> const char *name,
>  visit_type_int(v, name, &value, errp);
>  }
>  
> -static void pc_dimm_check_memdev_is_busy(Object *obj, const char *name,
> +static void pc_dimm_check_me

[Qemu-devel] [PULL 10/21] xics: directly register ICPState objects to vmstate

[David Gibson]

From: Greg Kurz 

The ICPState objects are currently registered to vmstate as qdev objects.
Their instance ids are hence computed automatically in the migration code,
and thus depends on the order the CPU cores were plugged.

If the destination had its CPU cores plugged in a different order than the
source, then ICPState objects will have different instance_ids and load
the wrong state.

Since CPU objects have a reliable cpu_index which is already used as
instance_id in vmstate, let's use it for ICPState as well.

Please note that this doesn't break migration. Older machine types used to
allocate and realize all ICPState objects at machine init time, for the whole
lifetime of the machine. The qdev instance ids are thus 0,1,2... nr_servers
and happen to map to the vCPU indexes.

Signed-off-by: Greg Kurz 
Reviewed-by: Laurent Vivier 
Signed-off-by: David Gibson 
---
 hw/intc/xics.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index d4194d6..a84ba51 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -344,10 +344,14 @@ static void icp_realize(DeviceState *dev, Error **errp)
 }
 
 qemu_register_reset(icp_reset, dev);
+vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
 }
 
 static void icp_unrealize(DeviceState *dev, Error **errp)
 {
+ICPState *icp = ICP(dev);
+
+vmstate_unregister(NULL, &vmstate_icp_server, icp);
 qemu_unregister_reset(icp_reset, dev);
 }
 
@@ -355,7 +359,6 @@ static void icp_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 
-dc->vmsd = &vmstate_icp_server;
 dc->realize = icp_realize;
 dc->unrealize = icp_unrealize;
 }
-- 
2.9.4

[Qemu-devel] [PULL 02/21] qapi: add explicit null to string input and output visitors

[David Gibson]

From: Greg Kurz 

This may be used for deprecated object properties that are kept for
backwards compatibility.

Signed-off-by: Greg Kurz 
Reviewed-by: Markus Armbruster 
Tested-by: Andrea Bolognani 
Signed-off-by: David Gibson 
---
 qapi/string-input-visitor.c  | 11 +++
 qapi/string-output-visitor.c | 14 ++
 2 files changed, 25 insertions(+)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index c089491..63ae115 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -326,6 +326,16 @@ static void parse_type_number(Visitor *v, const char 
*name, double *obj,
 *obj = val;
 }
 
+static void parse_type_null(Visitor *v, const char *name, Error **errp)
+{
+StringInputVisitor *siv = to_siv(v);
+
+if (!siv->string || siv->string[0]) {
+error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+   "null");
+}
+}
+
 static void string_input_free(Visitor *v)
 {
 StringInputVisitor *siv = to_siv(v);
@@ -349,6 +359,7 @@ Visitor *string_input_visitor_new(const char *str)
 v->visitor.type_bool = parse_type_bool;
 v->visitor.type_str = parse_type_str;
 v->visitor.type_number = parse_type_number;
+v->visitor.type_null = parse_type_null;
 v->visitor.start_list = start_list;
 v->visitor.next_list = next_list;
 v->visitor.check_list = check_list;
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 53c2175..af649e1 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -256,6 +256,19 @@ static void print_type_number(Visitor *v, const char 
*name, double *obj,
 string_output_set(sov, g_strdup_printf("%f", *obj));
 }
 
+static void print_type_null(Visitor *v, const char *name, Error **errp)
+{
+StringOutputVisitor *sov = to_sov(v);
+char *out;
+
+if (sov->human) {
+out = g_strdup("");
+} else {
+out = g_strdup("");
+}
+string_output_set(sov, out);
+}
+
 static void
 start_list(Visitor *v, const char *name, GenericList **list, size_t size,
Error **errp)
@@ -341,6 +354,7 @@ Visitor *string_output_visitor_new(bool human, char 
**result)
 v->visitor.type_bool = print_type_bool;
 v->visitor.type_str = print_type_str;
 v->visitor.type_number = print_type_number;
+v->visitor.type_null = print_type_null;
 v->visitor.start_list = start_list;
 v->visitor.next_list = next_list;
 v->visitor.end_list = end_list;
-- 
2.9.4

[Qemu-devel] [PULL 20/21] spapr: Clean up DRC set_allocation_state path

[David Gibson]

The allocation-state indicator should only actually be implemented for
"logical" DRCs, not physical ones.  Factor a check for this, and also for
valid indicator state values into rtas_set_allocation_state().  Because
they don't exist for physical DRCs, there's no reason that we'd ever want
more than one method implementation, so it can just be a plain function.

In addition, the setting to USABLE and setting to UNUSABLE paths in
set_allocation_state() don't actually have much in common.  So, split the
method separate functions for each parameter value (drc_set_usable()
and drc_set_unusable()).

Signed-off-by: David Gibson 
Reviewed-by: Greg Kurz 
Reviewed-by: Michael Roth 
---
 hw/ppc/spapr_drc.c | 73 --
 include/hw/ppc/spapr_drc.h |  2 --
 2 files changed, 45 insertions(+), 30 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 9a2511f..dbcd670 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -114,33 +114,42 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
 return RTAS_OUT_SUCCESS;
 }
 
-static uint32_t set_allocation_state(sPAPRDRConnector *drc,
- sPAPRDRAllocationState state)
+static uint32_t drc_set_usable(sPAPRDRConnector *drc)
 {
-trace_spapr_drc_set_allocation_state(spapr_drc_index(drc), state);
-
-if (state == SPAPR_DR_ALLOCATION_STATE_USABLE) {
-/* if there's no resource/device associated with the DRC, there's
- * no way for us to put it in an allocation state consistent with
- * being 'USABLE'. PAPR 2.7, 13.5.3.4 documents that this should
- * result in an RTAS return code of -3 / "no such indicator"
+/* if there's no resource/device associated with the DRC, there's
+ * no way for us to put it in an allocation state consistent with
+ * being 'USABLE'. PAPR 2.7, 13.5.3.4 documents that this should
+ * result in an RTAS return code of -3 / "no such indicator"
+ */
+if (!drc->dev) {
+return RTAS_OUT_NO_SUCH_INDICATOR;
+}
+if (drc->awaiting_release && drc->awaiting_allocation) {
+/* kernel is acknowledging a previous hotplug event
+ * while we are already removing it.
+ * it's safe to ignore awaiting_allocation here since we know the
+ * situation is predicated on the guest either already having done
+ * so (boot-time hotplug), or never being able to acquire in the
+ * first place (hotplug followed by immediate unplug).
  */
-if (!drc->dev) {
-return RTAS_OUT_NO_SUCH_INDICATOR;
-}
+return RTAS_OUT_NO_SUCH_INDICATOR;
 }
 
-if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
-drc->allocation_state = state;
-if (drc->awaiting_release &&
-drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
-uint32_t drc_index = spapr_drc_index(drc);
-trace_spapr_drc_set_allocation_state_finalizing(drc_index);
-spapr_drc_detach(drc, DEVICE(drc->dev), NULL);
-} else if (drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_USABLE) {
-drc->awaiting_allocation = false;
-}
+drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
+drc->awaiting_allocation = false;
+
+return RTAS_OUT_SUCCESS;
+}
+
+static uint32_t drc_set_unusable(sPAPRDRConnector *drc)
+{
+drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_UNUSABLE;
+if (drc->awaiting_release) {
+uint32_t drc_index = spapr_drc_index(drc);
+trace_spapr_drc_set_allocation_state_finalizing(drc_index);
+spapr_drc_detach(drc, DEVICE(drc->dev), NULL);
 }
+
 return RTAS_OUT_SUCCESS;
 }
 
@@ -547,7 +556,6 @@ static void spapr_dr_connector_class_init(ObjectClass *k, 
void *data)
 dk->realize = realize;
 dk->unrealize = unrealize;
 drck->set_isolation_state = set_isolation_state;
-drck->set_allocation_state = set_allocation_state;
 drck->release_pending = release_pending;
 /*
  * Reason: it crashes FIXME find and document the real reason
@@ -817,14 +825,23 @@ static uint32_t rtas_set_isolation_state(uint32_t idx, 
uint32_t state)
 static uint32_t rtas_set_allocation_state(uint32_t idx, uint32_t state)
 {
 sPAPRDRConnector *drc = spapr_drc_by_index(idx);
-sPAPRDRConnectorClass *drck;
 
-if (!drc) {
-return RTAS_OUT_PARAM_ERROR;
+if (!drc || !object_dynamic_cast(OBJECT(drc), TYPE_SPAPR_DRC_LOGICAL)) {
+return RTAS_OUT_NO_SUCH_INDICATOR;
 }
 
-drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-return drck->set_allocation_state(drc, state);
+trace_spapr_drc_set_allocation_state(spapr_drc_index(drc), state);
+
+switch (state) {
+case SPAPR_DR_ALLOCATION_STATE_USABLE:
+return drc_set_usable(drc);
+
+case SPAPR_DR_ALLOCATION_STATE_UNUSABLE:
+return drc_set_unusable(drc);
+
+default:
+return RTAS_OUT_PARAM_ERROR;
+}
 

[Qemu-devel] [PULL 01/21] hw/ppc/prep: Remove superfluous call to soundhw_init()

[David Gibson]

From: Thomas Huth 

When using the 40p machine, soundhw_init() is currently called twice,
one time from vl.c and one time from ibm_40p_init(). The call in
ibm_40p_init() was likely just a copy-and-paste from a old version
of the prep machine - but there the call to audio_init() (which was
the previous name of this function) has been removed many years ago
already, with commit b3e6d591b05538056d665572f3e3bbfb3cbb70e7
("audio: enable PCI audio cards for all PCI-enabled targets"), so
we certainly also do not need the soundhw_init() in the 40p function
anymore nowadays.

Signed-off-by: Thomas Huth 
Reviewed-by: Sahid Ferdjaoui 
Reviewed-by: Eduardo Habkost 
Reviewed-by: Hervé Poussineau 
Signed-off-by: David Gibson 
---
 hw/ppc/prep.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index d16646c..36d3dcd 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -36,7 +36,6 @@
 #include "hw/pci/pci_host.h"
 #include "hw/ppc/ppc.h"
 #include "hw/boards.h"
-#include "hw/audio/soundhw.h"
 #include "qemu/error-report.h"
 #include "qemu/log.h"
 #include "hw/ide.h"
@@ -782,9 +781,6 @@ static void ibm_40p_init(MachineState *machine)
 qbus_walk_children(BUS(isa_bus), prep_set_cmos_checksum, NULL, NULL, NULL,
&cmos_checksum);
 
-/* initialize audio subsystem */
-soundhw_init();
-
 /* add some more devices */
 if (defaults_enabled()) {
 isa_create_simple(isa_bus, "i8042");
-- 
2.9.4

[Qemu-devel] [PULL 07/21] spapr: Fix migration of Radix guests

[David Gibson]

From: Bharata B Rao 

Fix migration of radix guests by ensuring that we issue
KVM_PPC_CONFIGURE_V3_MMU for radix case post migration.

Reported-by: Nageswara R Sastry 
Signed-off-by: Bharata B Rao 
Reviewed-by: Suraj Jitindar Singh 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index f3e0b9b..6c276af 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1445,6 +1445,18 @@ static int spapr_post_load(void *opaque, int version_id)
 err = spapr_rtc_import_offset(&spapr->rtc, spapr->rtc_offset);
 }
 
+if (spapr->patb_entry) {
+PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+bool radix = !!(spapr->patb_entry & PATBE1_GR);
+bool gtse = !!(cpu->env.spr[SPR_LPCR] & LPCR_GTSE);
+
+err = kvmppc_configure_v3_mmu(cpu, radix, gtse, spapr->patb_entry);
+if (err) {
+error_report("Process table config unsupported by the host");
+return -EINVAL;
+}
+}
+
 return err;
 }
 
-- 
2.9.4

[Qemu-devel] [PULL 18/21] spapr: Split DRC release from DRC detach

[David Gibson]

spapr_drc_detach() is called when qemu generic code requests a device be
unplugged.  It makes a number of tests, which could well delay further
action until later, before actually detach the device from the DRC.

This splits out the part which actually removes the device from the DRC
into spapr_drc_release().  This will be useful for further cleanups.

Signed-off-by: David Gibson 
Reviewed-by: Greg Kurz 
Reviewed-by: Michael Roth 
---
 hw/ppc/spapr_drc.c | 49 +++--
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 21f5bf1..8a2b8f5 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -315,29 +315,8 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState 
*d, void *fdt,
  NULL, 0, NULL);
 }
 
-void spapr_drc_detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
+static void spapr_drc_release(sPAPRDRConnector *drc)
 {
-trace_spapr_drc_detach(spapr_drc_index(drc));
-
-if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) {
-trace_spapr_drc_awaiting_isolated(spapr_drc_index(drc));
-drc->awaiting_release = true;
-return;
-}
-
-if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
-drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
-trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc));
-drc->awaiting_release = true;
-return;
-}
-
-if (drc->awaiting_allocation) {
-drc->awaiting_release = true;
-trace_spapr_drc_awaiting_allocation(spapr_drc_index(drc));
-return;
-}
-
 drc->dr_indicator = SPAPR_DR_INDICATOR_INACTIVE;
 
 /* Calling release callbacks based on spapr_drc_type(drc). */
@@ -365,6 +344,32 @@ void spapr_drc_detach(sPAPRDRConnector *drc, DeviceState 
*d, Error **errp)
 drc->dev = NULL;
 }
 
+void spapr_drc_detach(sPAPRDRConnector *drc, DeviceState *d, Error **errp)
+{
+trace_spapr_drc_detach(spapr_drc_index(drc));
+
+if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) {
+trace_spapr_drc_awaiting_isolated(spapr_drc_index(drc));
+drc->awaiting_release = true;
+return;
+}
+
+if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
+drc->allocation_state != SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
+trace_spapr_drc_awaiting_unusable(spapr_drc_index(drc));
+drc->awaiting_release = true;
+return;
+}
+
+if (drc->awaiting_allocation) {
+drc->awaiting_release = true;
+trace_spapr_drc_awaiting_allocation(spapr_drc_index(drc));
+return;
+}
+
+spapr_drc_release(drc);
+}
+
 static bool release_pending(sPAPRDRConnector *drc)
 {
 return drc->awaiting_release;
-- 
2.9.4

[Qemu-devel] [PULL 00/21] ppc-for-2.10 queue 20170730

[David Gibson]

The following changes since commit c5eb5846d2d207bbde7f4b665d9ff90b92c8adff:

  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-hmp-20170629' into 
staging (2017-06-29 17:37:11 +0100)

are available in the git repository at:

  git://github.com/dgibson/qemu.git tags/ppc-for-2.10-20170630

for you to fetch changes up to 0dfabd39d523fc3f6f0f8c441f41c013cc429b52:

  spapr: Clean up DRC set_isolation_state() path (2017-06-30 14:03:32 +1000)


ppc patch queue 2017-06-30

  * More DRC cleanups, these now actually fix a few bugs
  * Properly implements the openpic timers (they now count and
generate interrupts)
  * Fixes for XICS migration
  * Fixes for migration of POWER9 RPT guests
  * The last of the compatibility mode rework


Aaron Larson (1):
  target-ppc: Enable open-pic timers to count and generate interrupts

Bharata B Rao (4):
  spapr: Add a "no HPT" encoding to HTAB migration stream
  spapr: Fix migration of Radix guests
  target/ppc: Proper cleanup when ppc_cpu_realizefn fails
  spapr: prevent QEMU crash when CPU realization fails

Daniel Henrique Barboza (1):
  hw/ppc/spapr.c: consecutive 'spapr->patb_entry = 0' statements

David Gibson (9):
  pseries: Move CPU compatibility property to machine
  pseries: Reset CPU compatibility mode
  ppc: Rework CPU compatibility testing across migration
  spapr: Start hotplugged PCI devices in ISOLATED state
  spapr: Eliminate DRC 'signalled' state variable
  spapr: Split DRC release from DRC detach
  spapr: Make DRC reset force DRC into known state
  spapr: Clean up DRC set_allocation_state path
  spapr: Clean up DRC set_isolation_state() path

Greg Kurz (3):
  qapi: add explicit null to string input and output visitors
  xics: directly register ICPState objects to vmstate
  spapr: fix migration of ICPState objects from/to older QEMU

Suraj Jitindar Singh (1):
  target/ppc: Fix return value in tcg radix mmu fault handler

Thomas Huth (2):
  hw/ppc/prep: Remove superfluous call to soundhw_init()
  target/ppc/excp_helper: Take BQL before calling cpu_interrupt()

 hw/intc/openpic.c| 117 +-
 hw/intc/xics.c   |   5 +-
 hw/ppc/prep.c|   4 -
 hw/ppc/spapr.c   | 160 ---
 hw/ppc/spapr_cpu_core.c  |  68 +++--
 hw/ppc/spapr_drc.c   | 355 +++
 hw/ppc/spapr_events.c|  10 --
 hw/ppc/spapr_hcall.c |   8 +-
 include/hw/ppc/spapr.h   |  13 +-
 include/hw/ppc/spapr_drc.h   |  10 +-
 qapi/string-input-visitor.c  |  11 ++
 qapi/string-output-visitor.c |  14 ++
 target/ppc/compat.c  | 102 +
 target/ppc/cpu.h |   6 +-
 target/ppc/excp_helper.c |   3 +
 target/ppc/machine.c |  69 -
 target/ppc/mmu-radix64.c |   2 +-
 target/ppc/translate_init.c  | 100 +---
 18 files changed, 749 insertions(+), 308 deletions(-)

[Qemu-devel] [PULL 09/21] target/ppc: Fix return value in tcg radix mmu fault handler

[David Gibson]

From: Suraj Jitindar Singh 

The mmu fault handler should return 0 if it was able to successfully
handle the fault and a positive value otherwise.

Currently the tcg radix mmu fault handler will return 1 after
successfully handling a fault in virtual mode. This is incorrect
so fix it so that it returns 0 in this case.

The handler already correctly returns 0 when a fault was handled
in real mode and 1 if an interrupt was generated.

Fixes: d5fee0bbe68d ("target/ppc: Implement ISA V3.00 radix page fault handler")

Signed-off-by: Suraj Jitindar Singh 
Signed-off-by: David Gibson 
---
 target/ppc/mmu-radix64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index de18c0b..69fde65 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -255,5 +255,5 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr 
eaddr, int rwx,
 
 tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
  prot, mmu_idx, 1UL << page_size);
-return 1;
+return 0;
 }
-- 
2.9.4

[Qemu-devel] [PULL 04/21] pseries: Reset CPU compatibility mode

[David Gibson]

Currently, the CPU compatibility mode is set when the cpu is initialized,
then again when the guest negotiates features.  This means if a guest
negotiates a compatibility mode, then reboots, that compatibility mode
will be retained across the reset.

Usually that will get overridden when features are negotiated on the next
boot, but it's still not really correct.  This patch moves the initial set
up of the compatibility mode from cpu init to reset time.  The mode *is*
retained if the reboot was caused by the feature negotiation (it might
be important in that case, though it's unlikely).

Signed-off-by: David Gibson 
Reviewed-by: Alexey Kardashevskiy 
Reviewed-by: Michael Roth 
Tested-by: Andrea Bolognani 
---
 hw/ppc/spapr.c  |  2 ++
 hw/ppc/spapr_cpu_core.c | 10 --
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 67d4c13..81007b9 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1346,6 +1346,8 @@ static void ppc_spapr_reset(void)
 if (!spapr->cas_reboot) {
 spapr_ovec_cleanup(spapr->ov5_cas);
 spapr->ov5_cas = spapr_ovec_new();
+
+ppc_set_compat_all(spapr->max_compat_pvr, &error_fatal);
 }
 
 fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 97472fd..d6719d5 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -118,16 +118,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu,
 /* Enable PAPR mode in TCG or KVM */
 cpu_ppc_set_papr(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
 
-if (spapr->max_compat_pvr) {
-Error *local_err = NULL;
-
-ppc_set_compat(cpu, spapr->max_compat_pvr, &local_err);
-if (local_err) {
-error_propagate(errp, local_err);
-return;
-}
-}
-
 qemu_register_reset(spapr_cpu_reset, cpu);
 spapr_cpu_reset(cpu);
 }
-- 
2.9.4

[Qemu-devel] [PULL 06/21] spapr: Add a "no HPT" encoding to HTAB migration stream

[David Gibson]

From: Bharata B Rao 

Add a "no HPT" encoding (using value -1) to the HTAB migration
stream (in the place of HPT size) when the guest doesn't allocate HPT.
This will help the target side to match target HPT with the source HPT
and thus enable successful migration.

Suggested-by: David Gibson 
Signed-off-by: Bharata B Rao 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 29 +
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 52f4e72..f3e0b9b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1560,13 +1560,19 @@ static int htab_save_setup(QEMUFile *f, void *opaque)
 sPAPRMachineState *spapr = opaque;
 
 /* "Iteration" header */
-qemu_put_be32(f, spapr->htab_shift);
+if (!spapr->htab_shift) {
+qemu_put_be32(f, -1);
+} else {
+qemu_put_be32(f, spapr->htab_shift);
+}
 
 if (spapr->htab) {
 spapr->htab_save_index = 0;
 spapr->htab_first_pass = true;
 } else {
-assert(kvm_enabled());
+if (spapr->htab_shift) {
+assert(kvm_enabled());
+}
 }
 
 
@@ -1712,7 +1718,12 @@ static int htab_save_iterate(QEMUFile *f, void *opaque)
 int rc = 0;
 
 /* Iteration header */
-qemu_put_be32(f, 0);
+if (!spapr->htab_shift) {
+qemu_put_be32(f, -1);
+return 0;
+} else {
+qemu_put_be32(f, 0);
+}
 
 if (!spapr->htab) {
 assert(kvm_enabled());
@@ -1746,7 +1757,12 @@ static int htab_save_complete(QEMUFile *f, void *opaque)
 int fd;
 
 /* Iteration header */
-qemu_put_be32(f, 0);
+if (!spapr->htab_shift) {
+qemu_put_be32(f, -1);
+return 0;
+} else {
+qemu_put_be32(f, 0);
+}
 
 if (!spapr->htab) {
 int rc;
@@ -1790,6 +1806,11 @@ static int htab_load(QEMUFile *f, void *opaque, int 
version_id)
 
 section_hdr = qemu_get_be32(f);
 
+if (section_hdr == -1) {
+spapr_free_hpt(spapr);
+return 0;
+}
+
 if (section_hdr) {
 Error *local_err = NULL;
 
-- 
2.9.4

[Qemu-devel] [PULL 08/21] target/ppc/excp_helper: Take BQL before calling cpu_interrupt()

[David Gibson]

From: Thomas Huth 

Since the introduction of MTTCG, using the msgsnd instruction
abort()s if being called without holding the BQL. So let's protect
that part of the code now with qemu_mutex_lock_iothread().

Buglink: https://bugs.launchpad.net/qemu/+bug/1694998
Signed-off-by: Thomas Huth 
Reviewed-by: Alex Bennée 
Signed-off-by: David Gibson 
---
 target/ppc/excp_helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 9cb2123..3a9f086 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see .
  */
 #include "qemu/osdep.h"
+#include "qemu/main-loop.h"
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
@@ -1132,6 +1133,7 @@ void helper_msgsnd(target_ulong rb)
 return;
 }
 
+qemu_mutex_lock_iothread();
 CPU_FOREACH(cs) {
 PowerPCCPU *cpu = POWERPC_CPU(cs);
 CPUPPCState *cenv = &cpu->env;
@@ -1141,5 +1143,6 @@ void helper_msgsnd(target_ulong rb)
 cpu_interrupt(cs, CPU_INTERRUPT_HARD);
 }
 }
+qemu_mutex_unlock_iothread();
 }
 #endif
-- 
2.9.4

[Qemu-devel] [PULL 19/21] spapr: Make DRC reset force DRC into known state

[David Gibson]

The reset handler for DRCs attempts several state transitions which are
subject to various checks and restrictions.  But at reset time we know
there is no guest, so we can ignore most of the usual sequencing rules and
just set the DRC back to a known state.  In fact, it's safer to do so.

The existing code also has several redundant checks for
drc->awaiting_release inside a block which has already tested that.  This
patch removes those and sets the DRC to a fixed initial state based only
on whether a device is currently plugged or not.

With DRCs correctly reset to a state based on device presence, we don't
need to force state transitions as cold plugged devices are processed.
This allows us to remove all the callers of the set_*_state() methods from
outside spapr_drc.c.

Signed-off-by: David Gibson 
Reviewed-by: Greg Kurz 
Reviewed-by: Michael Roth 
---
 hw/ppc/spapr.c | 15 ---
 hw/ppc/spapr_drc.c | 35 ---
 2 files changed, 16 insertions(+), 34 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3c12af0..0ee9fac 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2636,12 +2636,6 @@ static void spapr_add_lmbs(DeviceState *dev, uint64_t 
addr_start, uint64_t size,
 
 spapr_drc_attach(drc, dev, fdt, fdt_offset, !dev->hotplugged, errp);
 addr += SPAPR_MEMORY_BLOCK_SIZE;
-if (!dev->hotplugged) {
-sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-/* guests expect coldplugged LMBs to be pre-allocated */
-drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
-drck->set_isolation_state(drc, 
SPAPR_DR_ISOLATION_STATE_UNISOLATED);
-}
 }
 /* send hotplug notification to the
  * guest only in case of hotplugged memory
@@ -3009,15 +3003,6 @@ static void spapr_core_plug(HotplugHandler *hotplug_dev, 
DeviceState *dev,
  * of hotplugged CPUs.
  */
 spapr_hotplug_req_add_by_index(drc);
-} else {
-/*
- * Set the right DRC states for cold plugged CPU.
- */
-if (drc) {
-sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
-drck->set_allocation_state(drc, SPAPR_DR_ALLOCATION_STATE_USABLE);
-drck->set_isolation_state(drc, 
SPAPR_DR_ISOLATION_STATE_UNISOLATED);
-}
 }
 core_slot->cpu = OBJECT(dev);
 
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 8a2b8f5..9a2511f 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -378,7 +378,6 @@ static bool release_pending(sPAPRDRConnector *drc)
 static void reset(DeviceState *d)
 {
 sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
-sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 
 trace_spapr_drc_reset(spapr_drc_index(drc));
 
@@ -386,27 +385,25 @@ static void reset(DeviceState *d)
 drc->ccs = NULL;
 
 /* immediately upon reset we can safely assume DRCs whose devices
- * are pending removal can be safely removed, and that they will
- * subsequently be left in an ISOLATED state. move the DRC to this
- * state in these cases (which will in turn complete any pending
- * device removals)
+ * are pending removal can be safely removed.
  */
 if (drc->awaiting_release) {
-drck->set_isolation_state(drc, SPAPR_DR_ISOLATION_STATE_ISOLATED);
-/* generally this should also finalize the removal, but if the device
- * hasn't yet been configured we normally defer removal under the
- * assumption that this transition is taking place as part of device
- * configuration. so check if we're still waiting after this, and
- * force removal if we are
- */
-if (drc->awaiting_release) {
-spapr_drc_detach(drc, DEVICE(drc->dev), NULL);
-}
+spapr_drc_release(drc);
+}
+
+drc->awaiting_allocation = false;
 
-/* non-PCI devices may be awaiting a transition to UNUSABLE */
-if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI &&
-drc->awaiting_release) {
-drck->set_allocation_state(drc, 
SPAPR_DR_ALLOCATION_STATE_UNUSABLE);
+if (drc->dev) {
+/* A device present at reset is coldplugged */
+drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
+if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
+drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_USABLE;
+}
+} else {
+/* Otherwise device is absent, but might be hotplugged */
+drc->isolation_state = SPAPR_DR_ISOLATION_STATE_ISOLATED;
+if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
+drc->allocation_state = SPAPR_DR_ALLOCATION_STATE_UNUSABLE;
 }
 }
 }
-- 
2.9.4

[Qemu-devel] [PULL 05/21] ppc: Rework CPU compatibility testing across migration

[David Gibson]

Migrating between different CPU versions is a bit complicated for ppc.
A long time ago, we ensured identical CPU versions at either end by
checking the PVR had the same value.  However, this breaks under KVM
HV, because we always have to use the host's PVR - it's not
virtualized.  That would mean we couldn't migrate between hosts with
different PVRs, even if the CPUs are close enough to compatible in
practice (sometimes identical cores with different surrounding logic
have different PVRs, so this happens in practice quite often).

So, we removed the PVR check, but instead checked that several flags
indicating supported instructions matched.  This turns out to be a bad
idea, because those instruction masks are not architected information, but
essentially a TCG implementation detail.  So changes to qemu internal CPU
modelling can break migration - this happened between qemu-2.6 and
qemu-2.7.  That was addressed by 146c11f1 "target-ppc: Allow eventual
removal of old migration mistakes".

Now, verification of CPU compatibility across a migration basically doesn't
happen.  We simply ignore the PVR of the incoming migration, and hope the
cpu on the destination is close enough to work.

Now that we've cleaned up handling of processor compatibility modes
for pseries machine type, we can do better.  For new machine types
(pseries-2.10+) We allow migration if:

* The source and destination PVRs are for the same type of CPU, as
  determined by CPU class's pvr_match function
OR  * When the source was in a compatibility mode, and the destination CPU
  supports the same compatibility mode

For older machine types we retain the existing behaviour - current CAS
code will usually set a compat mode which would break backwards
migration if we made them use the new behaviour. [Fixed from an
earlier version by Greg Kurz].

Signed-off-by: David Gibson 
Signed-off-by: Greg Kurz 
Reviewed-by: Suraj Jitindar Singh 
Tested-by: Andrea Bolognani 
---
 hw/ppc/spapr.c  |  7 -
 target/ppc/cpu.h|  1 +
 target/ppc/machine.c| 69 +++--
 target/ppc/translate_init.c |  2 ++
 4 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 81007b9..52f4e72 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3362,7 +3362,12 @@ DEFINE_SPAPR_MACHINE(2_10, "2.10", true);
  * pseries-2.9
  */
 #define SPAPR_COMPAT_2_9   \
-HW_COMPAT_2_9
+HW_COMPAT_2_9  \
+{  \
+.driver = TYPE_POWERPC_CPU,\
+.property = "pre-2.10-migration",  \
+.value= "on",  \
+}, \
 
 static void spapr_machine_2_9_instance_options(MachineState *machine)
 {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 09393e6..6ee2a26 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1211,6 +1211,7 @@ struct PowerPCCPU {
 uint64_t mig_insns_flags;
 uint64_t mig_insns_flags2;
 uint32_t mig_nb_BATs;
+bool pre_2_10_migration;
 };
 
 static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 445f489..f578156 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -8,6 +8,7 @@
 #include "helper_regs.h"
 #include "mmu-hash64.h"
 #include "migration/cpu.h"
+#include "qapi/error.h"
 
 static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
 {
@@ -195,6 +196,27 @@ static void cpu_pre_save(void *opaque)
 }
 }
 
+/*
+ * Determine if a given PVR is a "close enough" match to the CPU
+ * object.  For TCG and KVM PR it would probably be sufficient to
+ * require an exact PVR match.  However for KVM HV the user is
+ * restricted to a PVR exactly matching the host CPU.  The correct way
+ * to handle this is to put the guest into an architected
+ * compatibility mode.  However, to allow a more forgiving transition
+ * and migration from before this was widely done, we allow migration
+ * between sufficiently similar PVRs, as determined by the CPU class's
+ * pvr_match() hook.
+ */
+static bool pvr_match(PowerPCCPU *cpu, uint32_t pvr)
+{
+PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+if (pvr == pcc->pvr) {
+return true;
+}
+return pcc->pvr_match(pcc, pvr);
+}
+
 static int cpu_post_load(void *opaque, int version_id)
 {
 PowerPCCPU *cpu = opaque;
@@ -203,10 +225,31 @@ static int cpu_post_load(void *opaque, int version_id)
 target_ulong msr;
 
 /*
- * We always ignore the source PVR. The user or management
- * software has to take care of running QEMU in a compatible mode.
+ * If we're operating in compat mode, we should be ok as long as
+ * the des

[Qemu-devel] [PULL 15/21] target-ppc: Enable open-pic timers to count and generate interrupts

[David Gibson]

From: Aaron Larson 

Previously QEMU open-pic implemented the 4 open-pic timers including
all timer registers, but the timers did not "count" or generate any
interrupts.  The patch makes the timers both count and generate
interrupts.  The timer clock frequency is fixed at 25MHZ.

--

Responding to V2 patch comments.
- Simplify clock frequency logic and commentary.
- Remove camelCase variables.
- Timer objects now created at init rather than lazily.

Signed-off-by: Aaron Larson 
Signed-off-by: David Gibson 
---
 hw/intc/openpic.c | 117 +++---
 1 file changed, 111 insertions(+), 6 deletions(-)

diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
index 5595bb2..9dd285b 100644
--- a/hw/intc/openpic.c
+++ b/hw/intc/openpic.c
@@ -45,6 +45,7 @@
 #include "qemu/bitops.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/log.h"
+#include "qemu/timer.h"
 
 //#define DEBUG_OPENPIC
 
@@ -54,8 +55,10 @@ static const int debug_openpic = 1;
 static const int debug_openpic = 0;
 #endif
 
+static int get_current_cpu(void);
 #define DPRINTF(fmt, ...) do { \
 if (debug_openpic) { \
+printf("Core%d: ", get_current_cpu()); \
 printf(fmt , ## __VA_ARGS__); \
 } \
 } while (0)
@@ -246,9 +249,31 @@ typedef struct IRQSource {
 #define IDR_EP  0x8000  /* external pin */
 #define IDR_CI  0x4000  /* critical interrupt */
 
+/* Convert between openpic clock ticks and nanosecs.  In the hardware the clock
+   frequency is driven by board inputs to the PIC which the PIC would then
+   divide by 4 or 8.  For now hard code to 25MZ.
+*/
+#define OPENPIC_TIMER_FREQ_MHZ 25
+#define OPENPIC_TIMER_NS_PER_TICK (1000 / OPENPIC_TIMER_FREQ_MHZ)
+static inline uint64_t ns_to_ticks(uint64_t ns)
+{
+return ns/ OPENPIC_TIMER_NS_PER_TICK;
+}
+static inline uint64_t ticks_to_ns(uint64_t ticks)
+{
+return ticks * OPENPIC_TIMER_NS_PER_TICK;
+}
+
 typedef struct OpenPICTimer {
 uint32_t tccr;  /* Global timer current count register */
 uint32_t tbcr;  /* Global timer base count register */
+int   n_IRQ;
+bool  qemu_timer_active; /* Is the qemu_timer is running? 
*/
+struct QEMUTimer *qemu_timer;
+struct OpenPICState  *opp;  /* Device timer is part of. */
+/* The QEMU_CLOCK_VIRTUAL time (in ns) corresponding to the last
+   current_count written or read, only defined if qemu_timer_active. */
+uint64_t  origin_time;
 } OpenPICTimer;
 
 typedef struct OpenPICMSI {
@@ -795,6 +820,65 @@ static uint64_t openpic_gbl_read(void *opaque, hwaddr 
addr, unsigned len)
 return retval;
 }
 
+static void openpic_tmr_set_tmr(OpenPICTimer *tmr, uint32_t val, bool enabled);
+
+static void qemu_timer_cb(void *opaque)
+{
+OpenPICTimer *tmr = opaque;
+OpenPICState *opp = tmr->opp;
+uint32_tn_IRQ = tmr->n_IRQ;
+uint32_t val =   tmr->tbcr & ~TBCR_CI;
+uint32_t tog = ((tmr->tccr & TCCR_TOG) ^ TCCR_TOG);  /* invert toggle. */
+
+DPRINTF("%s n_IRQ=%d\n", __func__, n_IRQ);
+/* Reload current count from base count and setup timer. */
+tmr->tccr = val | tog;
+openpic_tmr_set_tmr(tmr, val, /*enabled=*/true);
+/* Raise the interrupt. */
+opp->src[n_IRQ].destmask = read_IRQreg_idr(opp, n_IRQ);
+openpic_set_irq(opp, n_IRQ, 1);
+openpic_set_irq(opp, n_IRQ, 0);
+}
+
+/* If enabled is true, arranges for an interrupt to be raised val clocks into
+   the future, if enabled is false cancels the timer. */
+static void openpic_tmr_set_tmr(OpenPICTimer *tmr, uint32_t val, bool enabled)
+{
+uint64_t ns = ticks_to_ns(val & ~TCCR_TOG);
+/* A count of zero causes a timer to be set to expire immediately.  This
+   effectively stops the simulation since the timer is constantly expiring
+   which prevents guest code execution, so we don't honor that
+   configuration.  On real hardware, this situation would generate an
+   interrupt on every clock cycle if the interrupt was unmasked. */
+if ((ns == 0) || !enabled) {
+tmr->qemu_timer_active = false;
+tmr->tccr = tmr->tccr & TCCR_TOG;
+timer_del(tmr->qemu_timer); /* set timer to never expire. */
+} else {
+tmr->qemu_timer_active = true;
+uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+tmr->origin_time = now;
+timer_mod(tmr->qemu_timer, now + ns); /* set timer expiration. */
+}
+}
+
+/* Returns the currrent tccr value, i.e., timer value (in clocks) with
+   appropriate TOG. */
+static uint64_t openpic_tmr_get_timer(OpenPICTimer *tmr)
+{
+uint64_t retval;
+if (!tmr->qemu_timer_active) {
+retval = tmr->tccr;
+} else {
+uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+uint64_t used = now - tmr->origin_time;  /* nsecs */
+uint32_t used_ticks = (uint32_t)ns_to_ticks(used);
+uint32_t count = (tmr->tccr & ~TCCR_TOG) - used_ticks;
+retval 

[Qemu-devel] [PULL 11/21] spapr: fix migration of ICPState objects from/to older QEMU

[David Gibson]

From: Greg Kurz 

Commit 5bc8d26de20c ("spapr: allocate the ICPState object from under
sPAPRCPUCore") moved ICPState objects from the machine to CPU cores.
This is an improvement since we no longer allocate ICPState objects
that will never be used. But it has the side-effect of breaking
migration of older machine types from older QEMU versions.

This patch allows spapr to register dummy "icp/server" entries to vmstate.
These entries use a dedicated VMStateDescription that can swallow and
discard state of an incoming migration stream, and that don't send anything
on outgoing migration.

As for real ICPState objects, the instance_id is the cpu_index of the
corresponding vCPU, which happens to be equal to the generated instance_id
of older machine types.

The machine can unregister/register these entries when CPUs are dynamically
plugged/unplugged.

This is only available for pseries-2.9 and older machines, thanks to a
compat property.

Signed-off-by: Greg Kurz 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 88 --
 include/hw/ppc/spapr.h |  1 +
 2 files changed, 87 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6c276af..ef944f7 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -127,9 +127,49 @@ error:
 return NULL;
 }
 
+static bool pre_2_10_vmstate_dummy_icp_needed(void *opaque)
+{
+/* Dummy entries correspond to unused ICPState objects in older QEMUs,
+ * and newer QEMUs don't even have them. In both cases, we don't want
+ * to send anything on the wire.
+ */
+return false;
+}
+
+static const VMStateDescription pre_2_10_vmstate_dummy_icp = {
+.name = "icp/server",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = pre_2_10_vmstate_dummy_icp_needed,
+.fields = (VMStateField[]) {
+VMSTATE_UNUSED(4), /* uint32_t xirr */
+VMSTATE_UNUSED(1), /* uint8_t pending_priority */
+VMSTATE_UNUSED(1), /* uint8_t mfrr */
+VMSTATE_END_OF_LIST()
+},
+};
+
+static void pre_2_10_vmstate_register_dummy_icp(int i)
+{
+vmstate_register(NULL, i, &pre_2_10_vmstate_dummy_icp,
+ (void *)(uintptr_t) i);
+}
+
+static void pre_2_10_vmstate_unregister_dummy_icp(int i)
+{
+vmstate_unregister(NULL, &pre_2_10_vmstate_dummy_icp,
+   (void *)(uintptr_t) i);
+}
+
+static inline int xics_max_server_number(void)
+{
+return DIV_ROUND_UP(max_cpus * kvmppc_smt_threads(), smp_threads);
+}
+
 static void xics_system_init(MachineState *machine, int nr_irqs, Error **errp)
 {
 sPAPRMachineState *spapr = SPAPR_MACHINE(machine);
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
 
 if (kvm_enabled()) {
 if (machine_kernel_irqchip_allowed(machine) &&
@@ -151,6 +191,17 @@ static void xics_system_init(MachineState *machine, int 
nr_irqs, Error **errp)
 return;
 }
 }
+
+if (smc->pre_2_10_has_unused_icps) {
+int i;
+
+for (i = 0; i < xics_max_server_number(); i++) {
+/* Dummy entries get deregistered when real ICPState objects
+ * are registered during CPU core hotplug.
+ */
+pre_2_10_vmstate_register_dummy_icp(i);
+}
+}
 }
 
 static int spapr_fixup_cpu_smt_dt(void *fdt, int offset, PowerPCCPU *cpu,
@@ -979,7 +1030,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
 void *fdt;
 sPAPRPHBState *phb;
 char *buf;
-int smt = kvmppc_smt_threads();
 
 fdt = g_malloc0(FDT_MAX_SIZE);
 _FDT((fdt_create_empty_tree(fdt, FDT_MAX_SIZE)));
@@ -1019,7 +1069,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
 _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
 
 /* /interrupt controller */
-spapr_dt_xics(DIV_ROUND_UP(max_cpus * smt, smp_threads), fdt, 
PHANDLE_XICP);
+spapr_dt_xics(xics_max_server_number(), fdt, PHANDLE_XICP);
 
 ret = spapr_populate_memory(spapr, fdt);
 if (ret < 0) {
@@ -2845,9 +2895,24 @@ static void spapr_core_unplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
   Error **errp)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
+sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(ms);
 CPUCore *cc = CPU_CORE(dev);
 CPUArchId *core_slot = spapr_find_cpu_slot(ms, cc->core_id, NULL);
 
+if (smc->pre_2_10_has_unused_icps) {
+sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
+sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(cc));
+const char *typename = object_class_get_name(scc->cpu_class);
+size_t size = object_type_get_instance_size(typename);
+int i;
+
+for (i = 0; i < cc->nr_threads; i++) {
+CPUState *cs = CPU(sc->threads + i * size);
+
+pre_2_10_vmstate_register_dummy_icp(cs->cpu_index);
+}
+}
+
 assert(core_slot);
 core_slot->cpu = NULL;
 object_unparent(OBJECT(dev));
@@ -2899,6 +296

[Qemu-devel] [PULL 14/21] hw/ppc/spapr.c: consecutive 'spapr->patb_entry = 0' statements

[David Gibson]

From: Daniel Henrique Barboza 

In ppc_spapr_reset(), if the guest is using HPT, the code was executing:

} else {
spapr->patb_entry = 0;
spapr_setup_hpt_and_vrma(spapr);
}

And, at the end of spapr_setup_hpt_and_vrma:

/* We're setting up a hash table, so that means we're not radix */
spapr->patb_entry = 0;

Resulting in spapr->patb_entry being assigned to 0 twice in a row.

Given that 'spapr_setup_hpt_and_vrma' is also called inside
'spapr_check_setup_free_hpt' of spapr_hcall.c, this trivial patch removes
the 'patb_entry = 0' assignment from the 'else' clause inside ppc_spapr_reset
to avoid this behavior.

Signed-off-by: Daniel Henrique Barboza 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ef944f7..3c12af0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1376,7 +1376,6 @@ static void ppc_spapr_reset(void)
  * Set the GR bit in PATB so that we know there is no HPT. */
 spapr->patb_entry = PATBE1_GR;
 } else {
-spapr->patb_entry = 0;
 spapr_setup_hpt_and_vrma(spapr);
 }
 
-- 
2.9.4

[Qemu-devel] [PULL 03/21] pseries: Move CPU compatibility property to machine

[David Gibson]

Server class POWER CPUs have a "compat" property, which is used to set the
backwards compatibility mode for the processor.  However, this only makes
sense for machine types which don't give the guest access to hypervisor
privilege - otherwise the compatibility level is under the guest's control.

To reflect this, this removes the CPU 'compat' property and instead
creates a 'max-cpu-compat' property on the pseries machine.  Strictly
speaking this breaks compatibility, but AFAIK the 'compat' option was
never (directly) used with -device or device_add.

The option was used with -cpu.  So, to maintain compatibility, this
patch adds a hack to the cpu option parsing to strip out any compat
options supplied with -cpu and set them on the machine property
instead of the now deprecated cpu property.

Signed-off-by: David Gibson 
Tested-by: Suraj Jitindar Singh 
Reviewed-by: Greg Kurz 
Tested-by: Greg Kurz 
Tested-by: Andrea Bolognani 
---
 hw/ppc/spapr.c  |   6 ++-
 hw/ppc/spapr_cpu_core.c |  55 +++-
 hw/ppc/spapr_hcall.c|   8 ++--
 include/hw/ppc/spapr.h  |  12 --
 target/ppc/compat.c | 102 
 target/ppc/cpu.h|   5 ++-
 target/ppc/translate_init.c |  86 +++--
 7 files changed, 201 insertions(+), 73 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ea44358..67d4c13 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2131,7 +2131,7 @@ static void ppc_spapr_init(MachineState *machine)
 machine->cpu_model = kvm_enabled() ? "host" : smc->tcg_default_cpu;
 }
 
-ppc_cpu_parse_features(machine->cpu_model);
+spapr_cpu_parse_features(spapr);
 
 spapr_init_cpus(spapr);
 
@@ -2503,6 +2503,10 @@ static void spapr_machine_initfn(Object *obj)
 " place of standard EPOW events when 
possible"
 " (required for memory hot-unplug 
support)",
 NULL);
+
+ppc_compat_add_property(obj, "max-cpu-compat", &spapr->max_compat_pvr,
+"Maximum permitted CPU compatibility mode",
+&error_fatal);
 }
 
 static void spapr_machine_finalizefn(Object *obj)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 9fb896b..97472fd 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -20,6 +20,57 @@
 #include "sysemu/numa.h"
 #include "qemu/error-report.h"
 
+void spapr_cpu_parse_features(sPAPRMachineState *spapr)
+{
+/*
+ * Backwards compatibility hack:
+ *
+ *   CPUs had a "compat=" property which didn't make sense for
+ *   anything except pseries.  It was replaced by "max-cpu-compat"
+ *   machine option.  This supports old command lines like
+ *   -cpu POWER8,compat=power7
+ *   By stripping the compat option and applying it to the machine
+ *   before passing it on to the cpu level parser.
+ */
+gchar **inpieces;
+int i, j;
+gchar *compat_str = NULL;
+
+inpieces = g_strsplit(MACHINE(spapr)->cpu_model, ",", 0);
+
+/* inpieces[0] is the actual model string */
+i = 1;
+j = 1;
+while (inpieces[i]) {
+if (g_str_has_prefix(inpieces[i], "compat=")) {
+/* in case of multiple compat= options */
+g_free(compat_str);
+compat_str = inpieces[i];
+} else {
+j++;
+}
+
+i++;
+/* Excise compat options from list */
+inpieces[j] = inpieces[i];
+}
+
+if (compat_str) {
+char *val = compat_str + strlen("compat=");
+gchar *newprops = g_strjoinv(",", inpieces);
+
+object_property_set_str(OBJECT(spapr), val, "max-cpu-compat",
+&error_fatal);
+
+ppc_cpu_parse_features(newprops);
+g_free(newprops);
+} else {
+ppc_cpu_parse_features(MACHINE(spapr)->cpu_model);
+}
+
+g_strfreev(inpieces);
+}
+
 static void spapr_cpu_reset(void *opaque)
 {
 sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
@@ -67,10 +118,10 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, 
PowerPCCPU *cpu,
 /* Enable PAPR mode in TCG or KVM */
 cpu_ppc_set_papr(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
 
-if (cpu->max_compat) {
+if (spapr->max_compat_pvr) {
 Error *local_err = NULL;
 
-ppc_set_compat(cpu, cpu->max_compat, &local_err);
+ppc_set_compat(cpu, spapr->max_compat_pvr, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index aa1ffea..8624ce8 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1045,11 +1045,11 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
 }
 }
 
-static uint32_t cas_check_pvr(PowerPCCPU *cpu, target_ulong *addr,
-  Error

[Qemu-devel] [PULL 16/21] spapr: Start hotplugged PCI devices in ISOLATED state

[David Gibson]

PCI DRCs, and only PCI DRCs, are immediately moved to UNISOLATED isolation
state once the device is attached.  This has been there from the initial
implementation, and it's not clear why.

The state diagram in PAPR 13.4 suggests PCI devices should start in
ISOLATED state until the guest moves them into UNISOLATED, and the code in
the guest-side drmgr tool seems to work that way too.

Signed-off-by: David Gibson 
Reviewed-by: Michael Roth 
Reviewed-by: Greg Kurz 
---
 hw/ppc/spapr_drc.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 5cb75bb..91fc08d 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -304,16 +304,6 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState 
*d, void *fdt,
 }
 g_assert(fdt || coldplug);
 
-/* NOTE: setting initial isolation state to UNISOLATED means we can't
- * detach unless guest has a userspace/kernel that moves this state
- * back to ISOLATED in response to an unplug event, or this is done
- * manually by the admin prior. if we force things while the guest
- * may be accessing the device, we can easily crash the guest, so we
- * we defer completion of removal in such cases to the reset() hook.
- */
-if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_PCI) {
-drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
-}
 drc->dr_indicator = SPAPR_DR_INDICATOR_ACTIVE;
 
 drc->dev = d;
-- 
2.9.4

[Qemu-devel] [PULL 21/21] spapr: Clean up DRC set_isolation_state() path

[David Gibson]

There are substantial differences in the various paths through
set_isolation_state(), both for setting to ISOLATED versus UNISOLATED
state and for logical versus physical DRCs.

So, split the set_isolation_state() method into isolate() and unisolate()
methods, and give it different implementations for the two DRC types.

Factor some minimal common checks, including for valid indicator values
(which we weren't previously checking) into rtas_set_isolation_state().

Signed-off-by: David Gibson 
Reviewed-by: Greg Kurz 
Reviewed-by: Michael Roth 
---
 hw/ppc/spapr_drc.c | 145 -
 include/hw/ppc/spapr_drc.h |   6 +-
 2 files changed, 105 insertions(+), 46 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index dbcd670..bd40b84 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -46,30 +46,64 @@ uint32_t spapr_drc_index(sPAPRDRConnector *drc)
 | (drc->id & DRC_INDEX_ID_MASK);
 }
 
-static uint32_t set_isolation_state(sPAPRDRConnector *drc,
-sPAPRDRIsolationState state)
+static uint32_t drc_isolate_physical(sPAPRDRConnector *drc)
 {
-trace_spapr_drc_set_isolation_state(spapr_drc_index(drc), state);
-
 /* if the guest is configuring a device attached to this DRC, we
  * should reset the configuration state at this point since it may
  * no longer be reliable (guest released device and needs to start
  * over, or unplug occurred so the FDT is no longer valid)
  */
-if (state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
-g_free(drc->ccs);
-drc->ccs = NULL;
-}
+g_free(drc->ccs);
+drc->ccs = NULL;
 
-if (state == SPAPR_DR_ISOLATION_STATE_UNISOLATED) {
-/* cannot unisolate a non-existent resource, and, or resources
- * which are in an 'UNUSABLE' allocation state. (PAPR 2.7, 13.5.3.5)
- */
-if (!drc->dev ||
-drc->allocation_state == SPAPR_DR_ALLOCATION_STATE_UNUSABLE) {
-return RTAS_OUT_NO_SUCH_INDICATOR;
+drc->isolation_state = SPAPR_DR_ISOLATION_STATE_ISOLATED;
+
+/* if we're awaiting release, but still in an unconfigured state,
+ * it's likely the guest is still in the process of configuring
+ * the device and is transitioning the devices to an ISOLATED
+ * state as a part of that process. so we only complete the
+ * removal when this transition happens for a device in a
+ * configured state, as suggested by the state diagram from PAPR+
+ * 2.7, 13.4
+ */
+if (drc->awaiting_release) {
+uint32_t drc_index = spapr_drc_index(drc);
+if (drc->configured) {
+trace_spapr_drc_set_isolation_state_finalizing(drc_index);
+spapr_drc_detach(drc, DEVICE(drc->dev), NULL);
+} else {
+trace_spapr_drc_set_isolation_state_deferring(drc_index);
 }
 }
+drc->configured = false;
+
+return RTAS_OUT_SUCCESS;
+}
+
+static uint32_t drc_unisolate_physical(sPAPRDRConnector *drc)
+{
+/* cannot unisolate a non-existent resource, and, or resources
+ * which are in an 'UNUSABLE' allocation state. (PAPR 2.7,
+ * 13.5.3.5)
+ */
+if (!drc->dev) {
+return RTAS_OUT_NO_SUCH_INDICATOR;
+}
+
+drc->isolation_state = SPAPR_DR_ISOLATION_STATE_UNISOLATED;
+
+return RTAS_OUT_SUCCESS;
+}
+
+static uint32_t drc_isolate_logical(sPAPRDRConnector *drc)
+{
+/* if the guest is configuring a device attached to this DRC, we
+ * should reset the configuration state at this point since it may
+ * no longer be reliable (guest released device and needs to start
+ * over, or unplug occurred so the FDT is no longer valid)
+ */
+g_free(drc->ccs);
+drc->ccs = NULL;
 
 /*
  * Fail any requests to ISOLATE the LMB DRC if this LMB doesn't
@@ -81,35 +115,47 @@ static uint32_t set_isolation_state(sPAPRDRConnector *drc,
  * If the LMB being removed doesn't belong to a DIMM device that is
  * actually being unplugged, fail the isolation request here.
  */
-if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB) {
-if ((state == SPAPR_DR_ISOLATION_STATE_ISOLATED) &&
- !drc->awaiting_release) {
-return RTAS_OUT_HW_ERROR;
-}
+if (spapr_drc_type(drc) == SPAPR_DR_CONNECTOR_TYPE_LMB
+&& !drc->awaiting_release) {
+return RTAS_OUT_HW_ERROR;
 }
 
-drc->isolation_state = state;
+drc->isolation_state = SPAPR_DR_ISOLATION_STATE_ISOLATED;
 
-if (drc->isolation_state == SPAPR_DR_ISOLATION_STATE_ISOLATED) {
-/* if we're awaiting release, but still in an unconfigured state,
- * it's likely the guest is still in the process of configuring
- * the device and is transitioning the devices to an ISOLATED
- * state as a part of that process. so we only complete the
- * removal when this transition happens for a device in a
- * configured state, as sugge

[Qemu-devel] [PULL 13/21] spapr: prevent QEMU crash when CPU realization fails

[David Gibson]

From: Bharata B Rao 

ICPState objects were being allocated before CPU thread realization.
However commit 9ed656631d73 (xics: setup cpu at realize time) reversed it
by allocating ICPState objects after CPU thread is realized. But it
didn't take care to fix the error path because of which we observe
a SIGSEGV when CPU thread realization fails during cold/hotplug.

Fix this by ensuring that we do object_unparent() of ICPState object
only in case when is was created earlier.

Signed-off-by: Bharata B Rao 
Reviewed-by: Greg Kurz 
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_cpu_core.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index d6719d5..ea278ce 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -178,7 +178,7 @@ static void spapr_cpu_core_realize_child(Object *child, 
Error **errp)
 sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 CPUState *cs = CPU(child);
 PowerPCCPU *cpu = POWERPC_CPU(cs);
-Object *obj = NULL;
+Object *obj;
 
 object_property_set_bool(child, true, "realized", &local_err);
 if (local_err) {
@@ -198,13 +198,14 @@ static void spapr_cpu_core_realize_child(Object *child, 
Error **errp)
 object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort);
 object_property_set_bool(obj, true, "realized", &local_err);
 if (local_err) {
-goto error;
+goto free_icp;
 }
 
 return;
 
-error:
+free_icp:
 object_unparent(obj);
+error:
 error_propagate(errp, local_err);
 }
 
-- 
2.9.4

Re: [Qemu-devel] [PATCH v3 0/2] main_loop: Make main_loop_wait() return void

[Paolo Bonzini]

On 27/06/2017 19:32, Peter Maydell wrote:
> This patchset removes the useless return value from
> main_loop_wait().
> 
> Changes v2->v3:
>  * add initial patch which removes the use of the return value
>from the version of main_loop in test-char.c -- it didn't
>really need it anyway.
>  * removed unnecessary 'return;' at end of function
> 
> NB: I've just also noticed that there is exactly one use in the
> tree of the argument to main_loop_wait() now which doesn't
> pass 'false' -- that is in a function in hw/display/xenfb.c
> that was added 8 years ago with a comment claiming it was
> strictly temporary ;-)
> 
> 
> Peter Maydell (2):
>   tests/test-char.c: Don't use main_loop_wait()'s return value
>   main_loop: Make main_loop_wait() return void
> 
>  include/qemu/main-loop.h | 2 +-
>  tests/test-char.c| 6 +-
>  util/main-loop.c | 8 +---
>  3 files changed, 3 insertions(+), 13 deletions(-)
> 

Queued, thanks.

Paolo

[Qemu-devel] DIRTY_MEMORY_BLOCK_SIZE;

[ali saeedi]

Hello
what does 'DIRTY_MEMORY_BLOCK_SIZE' mean?
is it the number of words in a block? or number of pages in a block? or
number of bytes in a block?
thanks a lot

[Qemu-devel] [PULL 12/21] target/ppc: Proper cleanup when ppc_cpu_realizefn fails

[David Gibson]

From: Bharata B Rao 

If ppc_cpu_realizefn() fails after cpu_exec_realizefn() has been
called, we will have to undo whatever cpu_exec_realizefn() did
by explicitly calling cpu_exec_unrealizeffn() which is currently
missing. Failure to do this proper cleanup will result in CPU
which was never fully realized to linger on the cpus list causing
SIGSEGV later (for eg when running "info cpus").

Signed-off-by: Bharata B Rao 
Reviewed-by: Greg Kurz 
Signed-off-by: David Gibson 
---
 target/ppc/translate_init.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index ee84044..783bf98 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -9825,14 +9825,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 error_append_hint(errp, "Adjust the number of cpus to %d "
   "or try to raise the number of threads per core\n",
   cpu->cpu_dt_id * smp_threads / max_smt);
-return;
+goto unrealize;
 }
 #endif
 
 if (tcg_enabled()) {
 if (ppc_fixup_cpu(cpu) != 0) {
 error_setg(errp, "Unable to emulate selected CPU with TCG");
-return;
+goto unrealize;
 }
 }
 
@@ -9841,14 +9841,14 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 error_setg(errp, "CPU does not possess a BookE or 4xx MMU. "
"Please use qemu-system-ppc or qemu-system-ppc64 instead "
"or choose another CPU model.");
-return;
+goto unrealize;
 }
 #endif
 
 create_ppc_opcodes(cpu, &local_err);
 if (local_err != NULL) {
 error_propagate(errp, local_err);
-return;
+goto unrealize;
 }
 init_ppc_proc(cpu);
 
@@ -10033,6 +10033,10 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error 
**errp)
 fflush(stdout);
 }
 #endif
+return;
+
+unrealize:
+cpu_exec_unrealizefn(cs);
 }
 
 static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
-- 
2.9.4

Re: [Qemu-devel] [PULL 0/2] hmp queue

[Peter Maydell]

On 29 June 2017 at 17:30, Dr. David Alan Gilbert (git)
 wrote:
> From: "Dr. David Alan Gilbert" 
>
> The following changes since commit 454d7dc9bc13e46084e0612076e6952c40f4afeb:
>
>   Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging 
> (2017-06-29 16:21:45 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dagrh/qemu.git tags/pull-hmp-20170629
>
> for you to fetch changes up to bd1d5ad9f9a1347d6f4338f294253617c565c89a:
>
>   Add chardev-send-break monitor command (2017-06-29 17:14:11 +0100)
>
> 
> HMP pull 2017-06-29
>
> 
> Stefan Fritsch (1):
>   Add chardev-send-break monitor command
>
> Suraj Jitindar Singh (1):
>   monitor: Add -a (all) option to info registers
>
>  chardev/char.c   | 12 
>  hmp-commands-info.hx |  6 +++---
>  hmp-commands.hx  | 16 
>  hmp.c|  8 
>  hmp.h|  1 +
>  monitor.c| 21 -
>  qapi-schema.json | 20 
>  tests/test-char.c| 12 ++--
>  tests/test-hmp.c |  1 +
>  9 files changed, 87 insertions(+), 10 deletions(-)
>
Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v4 10/13] hmp: add hmp analogue for qmp-chardev-change

[Dr. David Alan Gilbert]

* Anton Nefedov (anton.nefe...@virtuozzo.com) wrote:
> Signed-off-by: Anton Nefedov 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> CC: Dr. David Alan Gilbert 

Acked-by: Dr. David Alan Gilbert 

> ---
>  include/chardev/char.h | 10 ++
>  hmp.h  |  1 +
>  chardev/char.c |  2 +-
>  hmp-commands.hx| 18 +-
>  hmp.c  | 34 ++
>  tests/test-hmp.c   |  1 +
>  6 files changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 22fd734..1604ea9 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -81,6 +81,16 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
>  void qemu_chr_parse_common(QemuOpts *opts, ChardevCommon *backend);
>  
>  /**
> + * @qemu_chr_parse_opts:
> + *
> + * Parse the options to the ChardevBackend struct.
> + *
> + * Returns: a new backend or NULL on error
> + */
> +ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
> +Error **errp);
> +
> +/**
>   * @qemu_chr_new:
>   *
>   * Create a new character backend from a URI.
> diff --git a/hmp.h b/hmp.h
> index d8b94ce..23e035c 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -102,6 +102,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict 
> *qdict);
>  void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
>  void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
>  void hmp_chardev_add(Monitor *mon, const QDict *qdict);
> +void hmp_chardev_change(Monitor *mon, const QDict *qdict);
>  void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
>  void hmp_qemu_io(Monitor *mon, const QDict *qdict);
>  void hmp_cpu_add(Monitor *mon, const QDict *qdict);
> diff --git a/chardev/char.c b/chardev/char.c
> index 6b2cb7b..8e8b881 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -567,7 +567,7 @@ static const char *chardev_alias_translate(const char 
> *name)
>  return name;
>  }
>  
> -static ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts, Error **errp)
> +ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts, Error **errp)
>  {
>  Error *local_err = NULL;
>  const ChardevClass *cc;
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e763606..2cad758 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1724,7 +1724,23 @@ ETEXI
>  STEXI
>  @item chardev-add args
>  @findex chardev-add
> -chardev_add accepts the same parameters as the -chardev command line switch.
> +chardev-add accepts the same parameters as the -chardev command line switch.
> +
> +ETEXI
> +
> +{
> +.name   = "chardev-change",
> +.args_type  = "id:s,args:s",
> +.params = "id args",
> +.help   = "change chardev",
> +.cmd= hmp_chardev_change,
> +},
> +
> +STEXI
> +@item chardev-change args
> +@findex chardev-change
> +chardev-change accepts existing chardev @var{id} and then the same arguments
> +as the -chardev command line switch (except for "id").
>  
>  ETEXI
>  
> diff --git a/hmp.c b/hmp.c
> index 8c72c58..91e4317 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2225,6 +2225,40 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict)
>  hmp_handle_error(mon, &err);
>  }
>  
> +void hmp_chardev_change(Monitor *mon, const QDict *qdict)
> +{
> +const char *args = qdict_get_str(qdict, "args");
> +const char *id;
> +Error *err = NULL;
> +ChardevBackend *backend = NULL;
> +ChardevReturn *ret = NULL;
> +QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"), args,
> + true);
> +if (!opts) {
> +error_setg(&err, "Parsing chardev args failed");
> +goto end;
> +}
> +
> +id = qdict_get_str(qdict, "id");
> +if (qemu_opts_id(opts)) {
> +error_setg(&err, "Unexpected 'id' parameter");
> +goto end;
> +}
> +
> +backend = qemu_chr_parse_opts(opts, &err);
> +if (!backend) {
> +goto end;
> +}
> +
> +ret = qmp_chardev_change(id, backend, &err);
> +
> +end:
> +qapi_free_ChardevReturn(ret);
> +qapi_free_ChardevBackend(backend);
> +qemu_opts_del(opts);
> +hmp_handle_error(mon, &err);
> +}
> +
>  void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
>  {
>  Error *local_err = NULL;
> diff --git a/tests/test-hmp.c b/tests/test-hmp.c
> index 99e35ec..299df19 100644
> --- a/tests/test-hmp.c
> +++ b/tests/test-hmp.c
> @@ -22,6 +22,7 @@ static int verbose;
>  static const char *hmp_cmds[] = {
>  "boot_set ndc",
>  "chardev-add null,id=testchardev1",
> +"chardev-change testchardev1 ringbuf",
>  "chardev-remove testchardev1",
>  "commit all",
>  "cpu-add 1",
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PULL 17/21] spapr: Eliminate DRC 'signalled' state variable

[David Gibson]

The 'signalled' field in the DRC appears to be entirely a torturous
workaround for the fact that PCI devices were started in UNISOLATED state
for unclear reasons.

1) 'signalled' is already meaningless for logical (so far, all non PCI)
DRCs.  It's always set to true (at least at any point it might be tested),
and can't be assigned any real meaning due to the way signalling works for
logical DRCs.

2) For PCI DRCs, the only time signalled would be false is when non-zero
functions of a multifunction device are hotplugged, followed by function
zero (the other way around is explicitly not permitted). In that case the
secondary function DRCs are attached, but the notification isn't sent to
the guest until function 0 is plugged.

3) signalled being false is used to allow a DRC detach to switch mode
back to ISOLATED state, which allows a secondary function to be hotplugged
then unplugged with function 0 never inserted.  Without this a secondary
function starting in UNISOLATED state couldn't be detached again without
function 0 being inserted, all the functions configured by the guest, then
sent back to ISOLATED state.

4) But now that PCI DRCs start in ISOLATED state, there's nothing to be
done.  If the guest doesn't get the notification, it won't switch the
device to UNISOLATED state, so nothing prevents it from being unplugged.
If the guest does move it to UNISOLATED state without the signal (due to
a manual drmgr call, for instance) then it really isn't safe to unplug it.

So, this patch removes the signalled variable and all code related to it.

Signed-off-by: David Gibson 
Reviewed-by: Greg Kurz 
Reviewed-by: Michael Roth 
---
 hw/ppc/spapr_drc.c | 45 +
 hw/ppc/spapr_events.c  | 10 --
 include/hw/ppc/spapr_drc.h |  2 --
 3 files changed, 1 insertion(+), 56 deletions(-)

diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 91fc08d..21f5bf1 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -172,12 +172,6 @@ static const char *spapr_drc_name(sPAPRDRConnector *drc)
 return g_strdup_printf("%s%d", drck->drc_name_prefix, drc->id);
 }
 
-/* has the guest been notified of device attachment? */
-static void set_signalled(sPAPRDRConnector *drc)
-{
-drc->signalled = true;
-}
-
 /*
  * dr-entity-sense sensor value
  * returned via get-sensor-state RTAS calls
@@ -310,17 +304,6 @@ void spapr_drc_attach(sPAPRDRConnector *drc, DeviceState 
*d, void *fdt,
 drc->fdt = fdt;
 drc->fdt_start_offset = fdt_start_offset;
 drc->configured = coldplug;
-/* 'logical' DR resources such as memory/cpus are in some cases treated
- * as a pool of resources from which the guest is free to choose from
- * based on only a count. for resources that can be assigned in this
- * fashion, we must assume the resource is signalled immediately
- * since a single hotplug request might make an arbitrary number of
- * such attached resources available to the guest, as opposed to
- * 'physical' DR resources such as PCI where each device/resource is
- * signalled individually.
- */
-drc->signalled = (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI)
- ? true : coldplug;
 
 if (spapr_drc_type(drc) != SPAPR_DR_CONNECTOR_TYPE_PCI) {
 drc->awaiting_allocation = true;
@@ -336,26 +319,6 @@ void spapr_drc_detach(sPAPRDRConnector *drc, DeviceState 
*d, Error **errp)
 {
 trace_spapr_drc_detach(spapr_drc_index(drc));
 
-/* if we've signalled device presence to the guest, or if the guest
- * has gone ahead and configured the device (via manually-executed
- * device add via drmgr in guest, namely), we need to wait
- * for the guest to quiesce the device before completing detach.
- * Otherwise, we can assume the guest hasn't seen it and complete the
- * detach immediately. Note that there is a small race window
- * just before, or during, configuration, which is this context
- * refers mainly to fetching the device tree via RTAS.
- * During this window the device access will be arbitrated by
- * associated DRC, which will simply fail the RTAS calls as invalid.
- * This is recoverable within guest and current implementations of
- * drmgr should be able to cope.
- */
-if (!drc->signalled && !drc->configured) {
-/* if the guest hasn't seen the device we can't rely on it to
- * set it back to an isolated state via RTAS, so do it here manually
- */
-drc->isolation_state = SPAPR_DR_ISOLATION_STATE_ISOLATED;
-}
-
 if (drc->isolation_state != SPAPR_DR_ISOLATION_STATE_ISOLATED) {
 trace_spapr_drc_awaiting_isolated(spapr_drc_index(drc));
 drc->awaiting_release = true;
@@ -441,10 +404,6 @@ static void reset(DeviceState *d)
 drck->set_allocation_state(drc, 
SPAPR_DR_ALLOCATION_STATE_UNUSABLE);
 }
 }
-
-if (drck->dr_entity_sense(drc) == SPAPR_DR_ENTITY_SENSE_PRESENT) {

Re: [Qemu-devel] [PATCH] vhost: Fix use-after-free in vhost_log_put()

[jsli]

On 2017-06-29 05:12, Marc-André Lureauwrote:
> Hi
>   
> On Fri, Jun 23, 2017 at 6:28 AM Jia-Shiun 
> Limailto:j...@synology.com)>wrote:
> > In commit 9e0bc24f dev->log_size was reset to zero too early before
> > syncing vhost log. It causes syncing to be skipped.
>   
> ooch, I guess I didn't realize it was also accessing dev->log_size when 
> taking dev->log in local variable.
> I wonder why the code is written this way, it looks like the function may be 
> reentered. For consistency, and perhaps for the reentering case, I would use 
> a local log_size variable too.
> Btw, how did you find this regression?
>   
>   
>   

Ok, it makes sense to prevent reentering. Willregenerate patch.
We are trying to do vhost-scsi migration, and found it to cause 
datainconsistency migrating an i/o stressed guest.

-Jia-Shiun

Re: [Qemu-devel] [PULL 00/21] ppc-for-2.10 queue 20170730

[Greg Kurz]

Not sure Sam's and Suraj's email addresses are correct but you also used
them in the "target/ppc/cpu-models: set POWER9_v1.0 as  POWER9 DD1" thread
and, strangely, I don't seem to receive 'unknow recipient' messages from
the Redhat MX servers... :)

Cheers,

--
Greg

On Fri, 30 Jun 2017 20:46:11 +1000
David Gibson  wrote:

> The following changes since commit c5eb5846d2d207bbde7f4b665d9ff90b92c8adff:
> 
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-hmp-20170629' into 
> staging (2017-06-29 17:37:11 +0100)
> 
> are available in the git repository at:
> 
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.10-20170630
> 
> for you to fetch changes up to 0dfabd39d523fc3f6f0f8c441f41c013cc429b52:
> 
>   spapr: Clean up DRC set_isolation_state() path (2017-06-30 14:03:32 +1000)
> 
> 
> ppc patch queue 2017-06-30
> 
>   * More DRC cleanups, these now actually fix a few bugs
>   * Properly implements the openpic timers (they now count and
> generate interrupts)
>   * Fixes for XICS migration
>   * Fixes for migration of POWER9 RPT guests
>   * The last of the compatibility mode rework
> 
> 
> Aaron Larson (1):
>   target-ppc: Enable open-pic timers to count and generate interrupts
> 
> Bharata B Rao (4):
>   spapr: Add a "no HPT" encoding to HTAB migration stream
>   spapr: Fix migration of Radix guests
>   target/ppc: Proper cleanup when ppc_cpu_realizefn fails
>   spapr: prevent QEMU crash when CPU realization fails
> 
> Daniel Henrique Barboza (1):
>   hw/ppc/spapr.c: consecutive 'spapr->patb_entry = 0' statements
> 
> David Gibson (9):
>   pseries: Move CPU compatibility property to machine
>   pseries: Reset CPU compatibility mode
>   ppc: Rework CPU compatibility testing across migration
>   spapr: Start hotplugged PCI devices in ISOLATED state
>   spapr: Eliminate DRC 'signalled' state variable
>   spapr: Split DRC release from DRC detach
>   spapr: Make DRC reset force DRC into known state
>   spapr: Clean up DRC set_allocation_state path
>   spapr: Clean up DRC set_isolation_state() path
> 
> Greg Kurz (3):
>   qapi: add explicit null to string input and output visitors
>   xics: directly register ICPState objects to vmstate
>   spapr: fix migration of ICPState objects from/to older QEMU
> 
> Suraj Jitindar Singh (1):
>   target/ppc: Fix return value in tcg radix mmu fault handler
> 
> Thomas Huth (2):
>   hw/ppc/prep: Remove superfluous call to soundhw_init()
>   target/ppc/excp_helper: Take BQL before calling cpu_interrupt()
> 
>  hw/intc/openpic.c| 117 +-
>  hw/intc/xics.c   |   5 +-
>  hw/ppc/prep.c|   4 -
>  hw/ppc/spapr.c   | 160 ---
>  hw/ppc/spapr_cpu_core.c  |  68 +++--
>  hw/ppc/spapr_drc.c   | 355 
> +++
>  hw/ppc/spapr_events.c|  10 --
>  hw/ppc/spapr_hcall.c |   8 +-
>  include/hw/ppc/spapr.h   |  13 +-
>  include/hw/ppc/spapr_drc.h   |  10 +-
>  qapi/string-input-visitor.c  |  11 ++
>  qapi/string-output-visitor.c |  14 ++
>  target/ppc/compat.c  | 102 +
>  target/ppc/cpu.h |   6 +-
>  target/ppc/excp_helper.c |   3 +
>  target/ppc/machine.c |  69 -
>  target/ppc/mmu-radix64.c |   2 +-
>  target/ppc/translate_init.c  | 100 +---
>  18 files changed, 749 insertions(+), 308 deletions(-)
> 



pgpllZHdbckq2.pgp
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] target-arm: v7M: ignore writes to CONTROL.SPSEL from Thread mode

[Peter Maydell]

For v7M, writes to the CONTROL register are only permitted for
privileged code. However even if the code is privileged, the
write must not affect the SPSEL bit in the CONTROL register
if the CPU is in Thread mode (as documented in the pseudocode
for the MSR instruction). Implement this, instead of permitting
SPSEL to be written in all cases.

This was causing mbed applications not to run, because the
RTX RTOS they use relies on this behaviour.

Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2594faa..4ed32c5 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8768,9 +8768,16 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, 
uint32_t val)
 }
 break;
 case 20: /* CONTROL */
-switch_v7m_sp(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 0);
-env->v7m.control = val & (R_V7M_CONTROL_SPSEL_MASK |
-  R_V7M_CONTROL_NPRIV_MASK);
+/* Writing to the SPSEL bit only has an effect if we are in
+ * thread mode; other bits can be updated by any privileged code.
+ * switch_v7m_sp() deals with updating the SPSEL bit in
+ * env->v7m.control, so we only need update the others.
+ */
+if (env->v7m.exception == 0) {
+switch_v7m_sp(env, (val & R_V7M_CONTROL_SPSEL_MASK) != 0);
+}
+env->v7m.control &= ~R_V7M_CONTROL_NPRIV_MASK;
+env->v7m.control |= val & R_V7M_CONTROL_NPRIV_MASK;
 break;
 default:
 qemu_log_mask(LOG_GUEST_ERROR, "Attempt to write unknown special"
-- 
2.7.4

Re: [Qemu-devel] DIRTY_MEMORY_BLOCK_SIZE;

[Dr. David Alan Gilbert]

* ali saeedi (ali.saeed...@gmail.com) wrote:
> Hello
> what does 'DIRTY_MEMORY_BLOCK_SIZE' mean?
> is it the number of words in a block? or number of pages in a block? or
> number of bytes in a block?
> thanks a lot

(cc'ing Stefan)
I think that DIRTY_MEMORY_BLOCK_SIZE is the number of TARGET_PAGEs
within one DIRTY_MEMORY_BLOCK
So with the common 4k target page that's 4k*256k*8=8GB/dirty memory
block - note these are just the size of structure sin qemu, it's still
got the ranularity ot mark individual target pages as dirty.

Dave

--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC 00/15] Error API: Flag errors in *errp even if errors are being ignored

[Markus Armbruster]

Eduardo Habkost  writes:

> On Thu, Jun 29, 2017 at 08:54:29AM +0200, Markus Armbruster wrote:
>> Eduardo Habkost  writes:
>> 
>> > On Wed, Jun 28, 2017 at 11:05:26AM +0200, Markus Armbruster wrote:
>> >> Eduardo Habkost  writes:
> [...]
>> >> > I understand the reason we need to support errp==NULL, as it
>> >> > makes life simpler for callers that don't want any extra error
>> >> > information.  However, this has the cost of making the functions
>> >> > that report errors more complex and error-prone.
>> >> >
>> >> > (Evidence of that: the 34 ERR_IS_* cases handled by the "use
>> >> > ERR_IS_* macros" patches in the series.  Where existing code will
>> >> > crash or behave differently if errp is NULL.)
>> >> 
>> >> Which of them could *not* use a suitable return value instead of *errp?
>> >
>> > I don't know.  But I'm not trying to improve those 34 ERR_IS_* cases.  I
>> > am trying to improve the 700+ functions that need the
>> > local_err/error_propagate() boilerplate code today.  This series already
>> > handles 346 of them automatically (see patch 14/15).
>> 
>> I agree the goal is reducing error_propagate() boilerplate.  I latched
>> onto the 34 ERR_IS_* cases only because you presented them as examples.
>
> The 34 ERR_IS_* cases were evidence of how easy it is to introduce
> mistakes with the current API.  Probably most of them are instances of
> (1) and (2) below.

The current interface can be abused, but how much abuse actually creeps
in?  I think we've been doing reasonably well there since we got rid of
the bad examples and improved documentation.

Moreover, the revised interface could also be abused.  Nothing stops you
from dereferencing errp before or after, the only thing that changes are
the examples people see in code.  I'm afraid the people who reinvent bad
examples from scratch despite the documentation telling them not to will
also bypass any macros the documentation tells them to use.

*Especially* if we use macros only sometimes.  ERR_IS_SET(&err) makes no
sense, so we'd still test err directly there, wouldn't we?

> [...]
>> >> > The Proposal
>> >> > 
>> >> >
>> >> > I'm proposing replacing NULL errp with a special macro:
>> >> > IGNORE_ERRORS.  The macro will trigger special behavior in the
>> >> > error API that will make it not save any error information in the
>> >> > error pointer, but still keep track of boolean error state in
>> >> > *errp.
>> >> >
>> >> > This will allow us to simplify the documented method to propagate errors
>> >> > from:
>> >> >
>> >> > Error *err = NULL;
>> >> > foo(arg, &err);
>> >> > if (err) {
>> >> > handle the error...
>> >> > error_propagate(errp, err);
>> >> > }
>> >> >
>> >> > to:
>> >> >
>> >> > foo(arg, errp);
>> >> > if (ERR_IS_SET(errp)) {
>> >> > handle the error...
>> >> > }
>> >> >
>> >> > This will allow us to stop using local_err variables and
>> >> > error_propagate() on hundreds of cases.
>> >> 
>> >> Getting rid of unnecessary local_err boilerplate is good.  The question
>> >> is how.  A possible alternative to your proposal is to follow GLib and
>> >> make functions return success/failure.
>> >> 
>> >> How do the two compare?
>> >
>> > This proposal proposal already gets rid of 346 error_propagate() calls
>> > automatically, and we probably can get rid of many others with
>> > additional Coccinelle scripts.
>> >
>> > Making functions return success/failure, on the other hand, would
>> > require rewriting them manually.
>> 
>> Yes, but how would the *result* compare?  I feel we should at least
>> explore this.  I'll try to find some time to play with it.
>
> I think the results of using the return value to indicate errors are
> possibly better.  But even on that case, I think ERR_IS_SET will be
> useful to avoid error_propagate() boilerplate until we convert all of
> them.

See my assessment below.

>> Coccinelle might let us automate some, but determining success
>> vs. failure will commonly require human smarts.
>> 
>> How many such functions we have?  Hmm...
>> 
>> @r@
>> identifier fun, errp;
>> typedef Error;
>> position p;
>> @@
>>  void fun(..., Error **errp)@p
>>  {
>>  ...
>>  }
>> @script:python@
>> p << r.p;
>> fun << r.fun;
>> @@
>> print("%s:%s:%s: %s()" % (p[0].file, p[0].line, p[0].column, fun))
>> 
>> Finds 1525.  Correcting the void mistake will be a huge pain, but
>> procrastinating can only make it grow.
>> 
>> >   This proposal doesn't even prevent
>> > that from happening.  I'd say it helps on that, because we can now find
>> > cases that still need to be converted by grepping for ERR_IS_SET.
>> 
>> I honestly doubt finding the cases is a problem.  We just grep for
>> error_propagate().
>
> True.  But I think finding low-hanging fruits will still be a problem. e.g.:
>
>   void f1(Error *errp);
>
>   void f2(Error *errp)
>   {
>   do_something();
> 

[Qemu-devel] [PULL 1/7] virtio-blk: trace vdev so devices can be distinguished

[Stefan Hajnoczi]

It is hard to analyze trace logs with multiple virtio-blk devices
because none of the trace events include the VirtIODevice *vdev.

This patch adds vdev so it's clear which device a request is associated
with.

I considered using VirtIOBlock *s instead but VirtIODevice *vdev is more
general and may be correlated with generic virtio trace events like
virtio_set_status.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Fam Zheng 
Message-id: 20170614092930.11234-1-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/block/virtio-blk.c | 12 +++-
 hw/block/trace-events | 10 +-
 2 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 604d37d..c0bd247 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -50,7 +50,7 @@ static void virtio_blk_req_complete(VirtIOBlockReq *req, 
unsigned char status)
 VirtIOBlock *s = req->dev;
 VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
-trace_virtio_blk_req_complete(req, status);
+trace_virtio_blk_req_complete(vdev, req, status);
 
 stb_p(&req->in->status, status);
 virtqueue_push(req->vq, &req->elem, req->in_len);
@@ -88,12 +88,13 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
 {
 VirtIOBlockReq *next = opaque;
 VirtIOBlock *s = next->dev;
+VirtIODevice *vdev = VIRTIO_DEVICE(s);
 
 aio_context_acquire(blk_get_aio_context(s->conf.conf.blk));
 while (next) {
 VirtIOBlockReq *req = next;
 next = req->mr_next;
-trace_virtio_blk_rw_complete(req, ret);
+trace_virtio_blk_rw_complete(vdev, req, ret);
 
 if (req->qiov.nalloc != -1) {
 /* If nalloc is != 1 req->qiov is a local copy of the original
@@ -355,7 +356,8 @@ static inline void submit_requests(BlockBackend *blk, 
MultiReqBuffer *mrb,
 mrb->reqs[i - 1]->mr_next = mrb->reqs[i];
 }
 
-trace_virtio_blk_submit_multireq(mrb, start, num_reqs,
+trace_virtio_blk_submit_multireq(VIRTIO_DEVICE(mrb->reqs[start]->dev),
+ mrb, start, num_reqs,
  sector_num << BDRV_SECTOR_BITS,
  qiov->size, is_write);
 block_acct_merge_done(blk_get_stats(blk),
@@ -526,11 +528,11 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 
 if (is_write) {
 qemu_iovec_init_external(&req->qiov, iov, out_num);
-trace_virtio_blk_handle_write(req, req->sector_num,
+trace_virtio_blk_handle_write(vdev, req, req->sector_num,
   req->qiov.size / BDRV_SECTOR_SIZE);
 } else {
 qemu_iovec_init_external(&req->qiov, in_iov, in_num);
-trace_virtio_blk_handle_read(req, req->sector_num,
+trace_virtio_blk_handle_read(vdev, req, req->sector_num,
  req->qiov.size / BDRV_SECTOR_SIZE);
 }
 
diff --git a/hw/block/trace-events b/hw/block/trace-events
index 65e83dc..c332c01 100644
--- a/hw/block/trace-events
+++ b/hw/block/trace-events
@@ -1,11 +1,11 @@
 # See docs/tracing.txt for syntax documentation.
 
 # hw/block/virtio-blk.c
-virtio_blk_req_complete(void *req, int status) "req %p status %d"
-virtio_blk_rw_complete(void *req, int ret) "req %p ret %d"
-virtio_blk_handle_write(void *req, uint64_t sector, size_t nsectors) "req %p 
sector %"PRIu64" nsectors %zu"
-virtio_blk_handle_read(void *req, uint64_t sector, size_t nsectors) "req %p 
sector %"PRIu64" nsectors %zu"
-virtio_blk_submit_multireq(void *mrb, int start, int num_reqs, uint64_t 
offset, size_t size, bool is_write) "mrb %p start %d num_reqs %d offset 
%"PRIu64" size %zu is_write %d"
+virtio_blk_req_complete(void *vdev, void *req, int status) "vdev %p req %p 
status %d"
+virtio_blk_rw_complete(void *vdev, void *req, int ret) "vdev %p req %p ret %d"
+virtio_blk_handle_write(void *vdev, void *req, uint64_t sector, size_t 
nsectors) "vdev %p req %p sector %"PRIu64" nsectors %zu"
+virtio_blk_handle_read(void *vdev, void *req, uint64_t sector, size_t 
nsectors) "vdev %p req %p sector %"PRIu64" nsectors %zu"
+virtio_blk_submit_multireq(void *vdev, void *mrb, int start, int num_reqs, 
uint64_t offset, size_t size, bool is_write) "vdev %p mrb %p start %d num_reqs 
%d offset %"PRIu64" size %zu is_write %d"
 
 # hw/block/hd-geometry.c
 hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p LCHS 
%d %d %d"
-- 
2.9.4

[Qemu-devel] [PULL 0/7] Block patches

[Stefan Hajnoczi]

The following changes since commit 464588675455afda2899e20a0b120e4075de50c7:

  Merge remote-tracking branch 'remotes/sstabellini/tags/xen-20170627-tag' into 
staging (2017-06-29 11:45:01 +0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to c324fd0a39cee43c13f6d1fb34f74fc5e2fb007b:

  virtio-pci: use ioeventfd even when KVM is disabled (2017-06-30 11:03:45 
+0100)





Stefan Hajnoczi (7):
  virtio-blk: trace vdev so devices can be distinguished
  libqos: fix typo in virtio.h QVirtQueue->used comment
  libqos: add virtio used ring support
  tests: fix virtio-scsi-test ISR dependence
  tests: fix virtio-blk-test ISR dependence
  tests: fix virtio-net-test ISR dependence
  virtio-pci: use ioeventfd even when KVM is disabled

 tests/libqos/virtio.h|  8 ++-
 hw/block/virtio-blk.c| 12 ++
 hw/virtio/virtio-pci.c   |  2 +-
 tests/libqos/virtio.c| 60 
 tests/virtio-blk-test.c  | 27 ++
 tests/virtio-net-test.c  |  6 ++---
 tests/virtio-scsi-test.c |  2 +-
 hw/block/trace-events| 10 
 8 files changed, 101 insertions(+), 26 deletions(-)

-- 
2.9.4

[Qemu-devel] [PULL 2/7] libqos: fix typo in virtio.h QVirtQueue->used comment

[Stefan Hajnoczi]

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Fam Zheng 
Tested-by: Eric Blake 
Tested-by: Kevin Wolf 
Message-id: 20170628184724.21378-2-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/virtio.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 3397a08..829de5e 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -26,7 +26,7 @@ typedef struct QVirtioDevice {
 typedef struct QVirtQueue {
 uint64_t desc; /* This points to an array of struct vring_desc */
 uint64_t avail; /* This points to a struct vring_avail */
-uint64_t used; /* This points to a struct vring_desc */
+uint64_t used; /* This points to a struct vring_used */
 uint16_t index;
 uint32_t size;
 uint32_t free_head;
-- 
2.9.4

[Qemu-devel] [PULL 3/7] libqos: add virtio used ring support

[Stefan Hajnoczi]

Existing tests do not touch the virtqueue used ring.  Instead they poll
the virtqueue ISR register and peek into their request's device-specific
status field.

It turns out that the virtqueue ISR register can be set to 1 more than
once for a single notification (see commit
83d768b5640946b7da55ce8335509df297e2c7cd "virtio: set ISR on dataplane
notifications").  This causes problems for tests that assume a 1:1
correspondence between the ISR being 1 and request completion.

Peeking at device-specific status fields is also problematic if the
device has no field that can be abused for EINPROGRESS polling
semantics.  This is the case if all the field's values may be set by the
device; there's no magic constant left for polling.

It's time to process the used ring for completed requests, just like a
real virtio guest driver.  This patch adds the necessary APIs.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Fam Zheng 
Tested-by: Eric Blake 
Tested-by: Kevin Wolf 
Message-id: 20170628184724.21378-3-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/libqos/virtio.h |  6 ++
 tests/libqos/virtio.c | 60 +++
 2 files changed, 66 insertions(+)

diff --git a/tests/libqos/virtio.h b/tests/libqos/virtio.h
index 829de5e..8fbcd18 100644
--- a/tests/libqos/virtio.h
+++ b/tests/libqos/virtio.h
@@ -32,6 +32,7 @@ typedef struct QVirtQueue {
 uint32_t free_head;
 uint32_t num_free;
 uint32_t align;
+uint16_t last_used_idx;
 bool indirect;
 bool event;
 } QVirtQueue;
@@ -120,6 +121,10 @@ uint8_t qvirtio_wait_status_byte_no_isr(QVirtioDevice *d,
 QVirtQueue *vq,
 uint64_t addr,
 gint64 timeout_us);
+void qvirtio_wait_used_elem(QVirtioDevice *d,
+QVirtQueue *vq,
+uint32_t desc_idx,
+gint64 timeout_us);
 void qvirtio_wait_config_isr(QVirtioDevice *d, gint64 timeout_us);
 QVirtQueue *qvirtqueue_setup(QVirtioDevice *d,
  QGuestAllocator *alloc, uint16_t index);
@@ -135,6 +140,7 @@ uint32_t qvirtqueue_add(QVirtQueue *vq, uint64_t data, 
uint32_t len, bool write,
 bool next);
 uint32_t qvirtqueue_add_indirect(QVirtQueue *vq, QVRingIndirectDesc *indirect);
 void qvirtqueue_kick(QVirtioDevice *d, QVirtQueue *vq, uint32_t free_head);
+bool qvirtqueue_get_buf(QVirtQueue *vq, uint32_t *desc_idx);
 
 void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t idx);
 #endif
diff --git a/tests/libqos/virtio.c b/tests/libqos/virtio.c
index ec30cb9..9880a69 100644
--- a/tests/libqos/virtio.c
+++ b/tests/libqos/virtio.c
@@ -116,6 +116,35 @@ uint8_t qvirtio_wait_status_byte_no_isr(QVirtioDevice *d,
 return val;
 }
 
+/*
+ * qvirtio_wait_used_elem:
+ * @desc_idx: The next expected vq->desc[] index in the used ring
+ * @timeout_us: How many microseconds to wait before failing
+ *
+ * This function waits for the next completed request on the used ring.
+ */
+void qvirtio_wait_used_elem(QVirtioDevice *d,
+QVirtQueue *vq,
+uint32_t desc_idx,
+gint64 timeout_us)
+{
+gint64 start_time = g_get_monotonic_time();
+
+for (;;) {
+uint32_t got_desc_idx;
+
+clock_step(100);
+
+if (d->bus->get_queue_isr_status(d, vq) &&
+qvirtqueue_get_buf(vq, &got_desc_idx)) {
+g_assert_cmpint(got_desc_idx, ==, desc_idx);
+return;
+}
+
+g_assert(g_get_monotonic_time() - start_time <= timeout_us);
+}
+}
+
 void qvirtio_wait_config_isr(QVirtioDevice *d, gint64 timeout_us)
 {
 gint64 start_time = g_get_monotonic_time();
@@ -272,6 +301,37 @@ void qvirtqueue_kick(QVirtioDevice *d, QVirtQueue *vq, 
uint32_t free_head)
 }
 }
 
+/*
+ * qvirtqueue_get_buf:
+ * @desc_idx: A pointer that is filled with the vq->desc[] index, may be NULL
+ *
+ * This function gets the next used element if there is one ready.
+ *
+ * Returns: true if an element was ready, false otherwise
+ */
+bool qvirtqueue_get_buf(QVirtQueue *vq, uint32_t *desc_idx)
+{
+uint16_t idx;
+
+idx = readw(vq->used + offsetof(struct vring_used, idx));
+if (idx == vq->last_used_idx) {
+return false;
+}
+
+if (desc_idx) {
+uint64_t elem_addr;
+
+elem_addr = vq->used +
+offsetof(struct vring_used, ring) +
+(vq->last_used_idx % vq->size) *
+sizeof(struct vring_used_elem);
+*desc_idx = readl(elem_addr + offsetof(struct vring_used_elem, id));
+}
+
+vq->last_used_idx++;
+return true;
+}
+
 void qvirtqueue_set_used_event(QVirtQueue *vq, uint16_t idx)
 {
 g_assert(vq->event);
-- 
2.9.4

[Qemu-devel] [PULL 5/7] tests: fix virtio-blk-test ISR dependence

[Stefan Hajnoczi]

Use the new used ring APIs instead of assuming ISR being set means the
request has completed.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Fam Zheng 
Tested-by: Eric Blake 
Tested-by: Kevin Wolf 
Message-id: 20170628184724.21378-5-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/virtio-blk-test.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index fd2078c..0576cb1 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -196,7 +196,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator 
*alloc,
 
 qvirtqueue_kick(dev, vq, free_head);
 
-qvirtio_wait_queue_isr(dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_BLK_TIMEOUT_US);
 status = readb(req_addr + 528);
 g_assert_cmpint(status, ==, 0);
 
@@ -218,7 +218,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator 
*alloc,
 
 qvirtqueue_kick(dev, vq, free_head);
 
-qvirtio_wait_queue_isr(dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_BLK_TIMEOUT_US);
 status = readb(req_addr + 528);
 g_assert_cmpint(status, ==, 0);
 
@@ -246,7 +246,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator 
*alloc,
 qvirtqueue_add(vq, req_addr + 528, 1, true, false);
 qvirtqueue_kick(dev, vq, free_head);
 
-qvirtio_wait_queue_isr(dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_BLK_TIMEOUT_US);
 status = readb(req_addr + 528);
 g_assert_cmpint(status, ==, 0);
 
@@ -267,7 +267,7 @@ static void test_basic(QVirtioDevice *dev, QGuestAllocator 
*alloc,
 
 qvirtqueue_kick(dev, vq, free_head);
 
-qvirtio_wait_queue_isr(dev, vq, QVIRTIO_BLK_TIMEOUT_US);
+qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_BLK_TIMEOUT_US);
 status = readb(req_addr + 528);
 g_assert_cmpint(status, ==, 0);
 
@@ -348,7 +348,7 @@ static void pci_indirect(void)
 free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
 qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
QVIRTIO_BLK_TIMEOUT_US);
 status = readb(req_addr + 528);
 g_assert_cmpint(status, ==, 0);
@@ -373,7 +373,7 @@ static void pci_indirect(void)
 free_head = qvirtqueue_add_indirect(&vqpci->vq, indirect);
 qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
QVIRTIO_BLK_TIMEOUT_US);
 status = readb(req_addr + 528);
 g_assert_cmpint(status, ==, 0);
@@ -484,7 +484,7 @@ static void pci_msix(void)
 qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
 qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
QVIRTIO_BLK_TIMEOUT_US);
 
 status = readb(req_addr + 528);
@@ -509,7 +509,7 @@ static void pci_msix(void)
 qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
 
-qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
QVIRTIO_BLK_TIMEOUT_US);
 
 status = readb(req_addr + 528);
@@ -540,6 +540,8 @@ static void pci_idx(void)
 uint64_t capacity;
 uint32_t features;
 uint32_t free_head;
+uint32_t write_head;
+uint32_t desc_idx;
 uint8_t status;
 char *data;
 
@@ -581,7 +583,8 @@ static void pci_idx(void)
 qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
 qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq, QVIRTIO_BLK_TIMEOUT_US);
+qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, free_head,
+   QVIRTIO_BLK_TIMEOUT_US);
 
 /* Write request */
 req.type = VIRTIO_BLK_T_OUT;
@@ -600,6 +603,7 @@ static void pci_idx(void)
 qvirtqueue_add(&vqpci->vq, req_addr + 16, 512, false, true);
 qvirtqueue_add(&vqpci->vq, req_addr + 528, 1, true, false);
 qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
+write_head = free_head;
 
 /* No notification expected */
 status = qvirtio_wait_status_byte_no_isr(&dev->vdev,
@@ -625,8 +629,11 @@ static void pci_idx(void)
 
 qvirtqueue_kick(&dev->vdev, &vqpci->vq, free_head);
 
-qvirtio_wait_queue_isr(&dev->vdev, &vqpci->vq,
+/* We get just one notification for both requests */
+qvirtio_wait_used_elem(&dev->vdev, &vqpci->vq, write_head,
QVIRTIO_BLK_TIMEOUT_US);
+g_assert(qvirtqueue_get_buf(&vqpci->vq, &desc_idx));
+g_assert_cmpint(desc_

[Qemu-devel] [PULL 7/7] virtio-pci: use ioeventfd even when KVM is disabled

[Stefan Hajnoczi]

Old kvm.ko versions only supported a tiny number of ioeventfds so
virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.

Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-blk-pci,iothread=iothread0 work even
when KVM is disabled.

I have tested that virtio-blk-pci works under TCG both with and without
iothread.

This patch fixes qemu-iotests 068, which was accidentally merged early
despite the dependency on ioeventfd.

Cc: Michael S. Tsirkin 
Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Fam Zheng 
Tested-by: Eric Blake 
Tested-by: Kevin Wolf 
Message-id: 20170628184724.21378-7-stefa...@redhat.com
Message-id: 20170615163813.7255-2-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 hw/virtio/virtio-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 20d6a08..301920e 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1740,7 +1740,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error 
**errp)
 bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
  !pci_bus_is_root(pci_dev->bus);
 
-if (!kvm_has_many_ioeventfds()) {
+if (kvm_enabled() && !kvm_has_many_ioeventfds()) {
 proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
 }
 
-- 
2.9.4

[Qemu-devel] [PULL 4/7] tests: fix virtio-scsi-test ISR dependence

[Stefan Hajnoczi]

Use the new used ring APIs instead of assuming ISR being set means the
request has completed.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Fam Zheng 
Tested-by: Eric Blake 
Tested-by: Kevin Wolf 
Message-id: 20170628184724.21378-4-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/virtio-scsi-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c
index eff71df..87a3b6e 100644
--- a/tests/virtio-scsi-test.c
+++ b/tests/virtio-scsi-test.c
@@ -121,7 +121,7 @@ static uint8_t virtio_scsi_do_command(QVirtIOSCSI *vs, 
const uint8_t *cdb,
 }
 
 qvirtqueue_kick(vs->dev, vq, free_head);
-qvirtio_wait_queue_isr(vs->dev, vq, QVIRTIO_SCSI_TIMEOUT_US);
+qvirtio_wait_used_elem(vs->dev, vq, free_head, QVIRTIO_SCSI_TIMEOUT_US);
 
 response = readb(resp_addr +
  offsetof(struct virtio_scsi_cmd_resp, response));
-- 
2.9.4

[Qemu-devel] [PULL 6/7] tests: fix virtio-net-test ISR dependence

[Stefan Hajnoczi]

Use the new used ring APIs instead of assuming ISR being set means the
request has completed.

Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Fam Zheng 
Tested-by: Eric Blake 
Tested-by: Kevin Wolf 
Message-id: 20170628184724.21378-6-stefa...@redhat.com
Signed-off-by: Stefan Hajnoczi 
---
 tests/virtio-net-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 8f94360..635b942 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -108,7 +108,7 @@ static void rx_test(QVirtioDevice *dev,
 ret = iov_send(socket, iov, 2, 0, sizeof(len) + sizeof(test));
 g_assert_cmpint(ret, ==, sizeof(test) + sizeof(len));
 
-qvirtio_wait_queue_isr(dev, vq, QVIRTIO_NET_TIMEOUT_US);
+qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_NET_TIMEOUT_US);
 memread(req_addr + VNET_HDR_SIZE, buffer, sizeof(test));
 g_assert_cmpstr(buffer, ==, "TEST");
 
@@ -131,7 +131,7 @@ static void tx_test(QVirtioDevice *dev,
 free_head = qvirtqueue_add(vq, req_addr, 64, false, false);
 qvirtqueue_kick(dev, vq, free_head);
 
-qvirtio_wait_queue_isr(dev, vq, QVIRTIO_NET_TIMEOUT_US);
+qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_NET_TIMEOUT_US);
 guest_free(alloc, req_addr);
 
 ret = qemu_recv(socket, &len, sizeof(len), 0);
@@ -182,7 +182,7 @@ static void rx_stop_cont_test(QVirtioDevice *dev,
 rsp = qmp("{ 'execute' : 'cont'}");
 QDECREF(rsp);
 
-qvirtio_wait_queue_isr(dev, vq, QVIRTIO_NET_TIMEOUT_US);
+qvirtio_wait_used_elem(dev, vq, free_head, QVIRTIO_NET_TIMEOUT_US);
 memread(req_addr + VNET_HDR_SIZE, buffer, sizeof(test));
 g_assert_cmpstr(buffer, ==, "TEST");
 
-- 
2.9.4

[Qemu-devel] Page Block word

[ali saeedi]

Hello
what is the difference between word, page and block in qemu?
thanks a lot

[Qemu-devel] [PATCH v2] vhost: Fix use-after-free in vhost_log_put()

[Jia-Shiun Li]

In commit 9e0bc24f dev->log_size was reset to zero too early before
syncing vhost log. It causes syncing to be skipped.

Use local variable to keep its value before resetting.

Signed-off-by: Jia-Shiun Li 
---

v1 -> v2:
 * Use local variable to keep value of dev->log_size.

---
 hw/virtio/vhost.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 6eddb09..c1c5714 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -371,6 +371,7 @@ static struct vhost_log *vhost_log_get(uint64_t size, bool 
share)
 static void vhost_log_put(struct vhost_dev *dev, bool sync)
 {
 struct vhost_log *log = dev->log;
+uint64_t log_size = dev->log_size;
 
 if (!log) {
 return;
@@ -381,8 +382,8 @@ static void vhost_log_put(struct vhost_dev *dev, bool sync)
 --log->refcnt;
 if (log->refcnt == 0) {
 /* Sync only the range covered by the old log */
-if (dev->log_size && sync) {
-vhost_log_sync_range(dev, 0, dev->log_size * VHOST_LOG_CHUNK - 1);
+if (log_size && sync) {
+vhost_log_sync_range(dev, 0, log_size * VHOST_LOG_CHUNK - 1);
 }
 
 if (vhost_log == log) {
-- 
2.7.4

Re: [Qemu-devel] [PATCH v2] vhost: Fix use-after-free in vhost_log_put()

[Marc-André Lureau]

On Fri, Jun 30, 2017 at 2:03 PM Jia-Shiun Li  wrote:

> In commit 9e0bc24f dev->log_size was reset to zero too early before
> syncing vhost log. It causes syncing to be skipped.
>
> Use local variable to keep its value before resetting.
>
> Signed-off-by: Jia-Shiun Li 
>

 Reviewed-by: Marc-André Lureau 

---
>
> v1 -> v2:
>  * Use local variable to keep value of dev->log_size.
>
> ---
>  hw/virtio/vhost.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 6eddb09..c1c5714 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -371,6 +371,7 @@ static struct vhost_log *vhost_log_get(uint64_t size,
> bool share)
>  static void vhost_log_put(struct vhost_dev *dev, bool sync)
>  {
>  struct vhost_log *log = dev->log;
> +uint64_t log_size = dev->log_size;
>
>  if (!log) {
>  return;
> @@ -381,8 +382,8 @@ static void vhost_log_put(struct vhost_dev *dev, bool
> sync)
>  --log->refcnt;
>  if (log->refcnt == 0) {
>  /* Sync only the range covered by the old log */
> -if (dev->log_size && sync) {
> -vhost_log_sync_range(dev, 0, dev->log_size * VHOST_LOG_CHUNK
> - 1);
> +if (log_size && sync) {
> +vhost_log_sync_range(dev, 0, log_size * VHOST_LOG_CHUNK - 1);
>  }
>
>  if (vhost_log == log) {
> --
> 2.7.4
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PULL 00/21] ppc-for-2.10 queue 20170730

[Peter Maydell]

On 30 June 2017 at 11:46, David Gibson  wrote:
> The following changes since commit c5eb5846d2d207bbde7f4b665d9ff90b92c8adff:
>
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-hmp-20170629' into 
> staging (2017-06-29 17:37:11 +0100)
>
> are available in the git repository at:
>
>   git://github.com/dgibson/qemu.git tags/ppc-for-2.10-20170630
>
> for you to fetch changes up to 0dfabd39d523fc3f6f0f8c441f41c013cc429b52:
>
>   spapr: Clean up DRC set_isolation_state() path (2017-06-30 14:03:32 +1000)
>
> 
> ppc patch queue 2017-06-30
>
>   * More DRC cleanups, these now actually fix a few bugs
>   * Properly implements the openpic timers (they now count and
> generate interrupts)
>   * Fixes for XICS migration
>   * Fixes for migration of POWER9 RPT guests
>   * The last of the compatibility mode rework
>

Applied, thanks.

-- PMM

[Qemu-devel] [PULL 1/7] target/m68k: add fscc.

[Laurent Vivier]

use DisasCompare with FPU conditions in fscc and fbcc.

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Message-Id: <20170628204241.32106-2-laur...@vivier.eu>
---
 target/m68k/translate.c | 210 ++--
 1 file changed, 131 insertions(+), 79 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 7aa0fdc..5f4bedc 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4633,142 +4633,193 @@ undef:
 disas_undef_fpu(env, s, insn);
 }
 
-DISAS_INSN(fbcc)
+static void gen_fcc_cond(DisasCompare *c, DisasContext *s, int cond)
 {
-uint32_t offset;
-uint32_t addr;
-TCGLabel *l1;
-TCGv tmp, fpsr;
-
-addr = s->pc;
-offset = cpu_ldsw_code(env, s->pc);
-s->pc += 2;
-if (insn & (1 << 6)) {
-offset = (offset << 16) | read_im16(env, s);
-}
+TCGv fpsr;
 
+c->g1 = 1;
+c->v2 = tcg_const_i32(0);
+c->g2 = 0;
+/* TODO: Raise BSUN exception.  */
 fpsr = tcg_temp_new();
 gen_load_fcr(s, fpsr, M68K_FPSR);
-l1 = gen_new_label();
-/* TODO: Raise BSUN exception.  */
-/* Jump to l1 if condition is true.  */
-switch (insn & 0x3f)  {
+switch (cond) {
 case 0:  /* False */
 case 16: /* Signaling False */
+c->v1 = c->v2;
+c->tcond = TCG_COND_NEVER;
 break;
 case 1:  /* EQual Z */
 case 17: /* Signaling EQual Z */
-tmp = tcg_temp_new();
-tcg_gen_andi_i32(tmp, fpsr, FPSR_CC_Z);
-tcg_gen_brcondi_i32(TCG_COND_NE, tmp, 0, l1);
+c->v1 = tcg_temp_new();
+c->g1 = 0;
+tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_Z);
+c->tcond = TCG_COND_NE;
 break;
 case 2:  /* Ordered Greater Than !(A || Z || N) */
 case 18: /* Greater Than !(A || Z || N) */
-tmp = tcg_temp_new();
-tcg_gen_andi_i32(tmp, fpsr,
+c->v1 = tcg_temp_new();
+c->g1 = 0;
+tcg_gen_andi_i32(c->v1, fpsr,
  FPSR_CC_A | FPSR_CC_Z | FPSR_CC_N);
-tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, l1);
+c->tcond = TCG_COND_EQ;
 break;
 case 3:  /* Ordered Greater than or Equal Z || !(A || N) */
 case 19: /* Greater than or Equal Z || !(A || N) */
-assert(FPSR_CC_A == (FPSR_CC_N >> 3));
-tmp = tcg_temp_new();
-tcg_gen_shli_i32(tmp, fpsr, 3);
-tcg_gen_or_i32(tmp, tmp, fpsr);
-tcg_gen_xori_i32(tmp, tmp, FPSR_CC_N);
-tcg_gen_andi_i32(tmp, tmp, FPSR_CC_N | FPSR_CC_Z);
-tcg_gen_brcondi_i32(TCG_COND_NE, tmp, 0, l1);
+c->v1 = tcg_temp_new();
+c->g1 = 0;
+tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_A);
+tcg_gen_shli_i32(c->v1, c->v1, ctz32(FPSR_CC_N) - ctz32(FPSR_CC_A));
+tcg_gen_andi_i32(fpsr, fpsr, FPSR_CC_Z | FPSR_CC_N);
+tcg_gen_or_i32(c->v1, c->v1, fpsr);
+tcg_gen_xori_i32(c->v1, c->v1, FPSR_CC_N);
+c->tcond = TCG_COND_NE;
 break;
 case 4:  /* Ordered Less Than !(!N || A || Z); */
 case 20: /* Less Than !(!N || A || Z); */
-tmp = tcg_temp_new();
-tcg_gen_xori_i32(tmp, fpsr, FPSR_CC_N);
-tcg_gen_andi_i32(tmp, tmp, FPSR_CC_N | FPSR_CC_A | FPSR_CC_Z);
-tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, l1);
+c->v1 = tcg_temp_new();
+c->g1 = 0;
+tcg_gen_xori_i32(c->v1, fpsr, FPSR_CC_N);
+tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_N | FPSR_CC_A | FPSR_CC_Z);
+c->tcond = TCG_COND_EQ;
 break;
 case 5:  /* Ordered Less than or Equal Z || (N && !A) */
 case 21: /* Less than or Equal Z || (N && !A) */
-assert(FPSR_CC_A == (FPSR_CC_N >> 3));
-tmp = tcg_temp_new();
-tcg_gen_xori_i32(tmp, fpsr, FPSR_CC_A);
-tcg_gen_shli_i32(tmp, tmp, 3);
-tcg_gen_ori_i32(tmp, tmp, FPSR_CC_Z);
-tcg_gen_and_i32(tmp, tmp, fpsr);
-tcg_gen_brcondi_i32(TCG_COND_NE, tmp, 0, l1);
+c->v1 = tcg_temp_new();
+c->g1 = 0;
+tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_A);
+tcg_gen_shli_i32(c->v1, c->v1, ctz32(FPSR_CC_N) - ctz32(FPSR_CC_A));
+tcg_gen_andc_i32(c->v1, fpsr, c->v1);
+tcg_gen_andi_i32(c->v1, c->v1, FPSR_CC_Z | FPSR_CC_N);
+c->tcond = TCG_COND_NE;
 break;
 case 6:  /* Ordered Greater or Less than !(A || Z) */
 case 22: /* Greater or Less than !(A || Z) */
-tmp = tcg_temp_new();
-tcg_gen_andi_i32(tmp, fpsr, FPSR_CC_A | FPSR_CC_Z);
-tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, l1);
+c->v1 = tcg_temp_new();
+c->g1 = 0;
+tcg_gen_andi_i32(c->v1, fpsr, FPSR_CC_A | FPSR_CC_Z);
+c->tcond = TCG_COND_EQ;
 break;
 case 7:  /* Ordered !A */
 case 23: /* Greater, Less or Equal !A */
-tmp = tcg_temp_new();
-tcg_gen_andi_i32(tmp, fpsr, FPSR_CC_A);
-tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, 0, l1);
+c->v1 = tcg_temp_new();
+c->g1 = 0;
+tcg_gen_

[Qemu-devel] [PULL 0/7] M68k for 2.10 patches

[Laurent Vivier]

The following changes since commit 4c8c1cc544dbd5e2564868e61c5037258e393832:

  Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-2.10-pull-request' 
into staging (2017-06-22 19:01:58 +0100)

are available in the git repository at:

  git://github.com/vivier/qemu-m68k.git tags/m68k-for-2.10-pull-request

for you to fetch changes up to a1e58ddcb3eed7ec4a158512b9dae46f90492c1b:

  target/m68k: add fmovem (2017-06-29 20:29:57 +0200)





Laurent Vivier (7):
  target/m68k: add fscc.
  target/m68k: add fmovecr
  target/m68k: add explicit single and double precision operations
  softfloat: define floatx80_round()
  target/m68k: add fsglmul and fsgldiv
  target/m68k: add explicit single and double precision operations (part
2)
  target/m68k: add fmovem

 fpu/softfloat.c  |  16 ++
 include/fpu/softfloat.h  |   1 +
 target/m68k/fpu_helper.c | 310 -
 target/m68k/helper.h |  27 +++-
 target/m68k/translate.c  | 388 ---
 5 files changed, 619 insertions(+), 123 deletions(-)

-- 
2.9.4

[Qemu-devel] [PULL 6/7] target/m68k: add explicit single and double precision operations (part 2)

[Laurent Vivier]

Add fsabs, fdabs, fsneg, fdneg, fsmove and fdmove.

The value is converted using the new floatx80_round() function.

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Message-Id: <20170628204241.32106-7-laur...@vivier.eu>
---
 target/m68k/fpu_helper.c | 48 +---
 target/m68k/helper.h |  8 +++-
 target/m68k/translate.c  | 26 ++
 3 files changed, 74 insertions(+), 8 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 600ae8a..382fbc1 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -167,6 +167,20 @@ void HELPER(set_fpcr)(CPUM68KState *env, uint32_t val)
 set_floatx80_rounding_precision(old, &env->fp_status);  \
 } while (0)
 
+void HELPER(fsround)(CPUM68KState *env, FPReg *res, FPReg *val)
+{
+PREC_BEGIN(32);
+res->d = floatx80_round(val->d, &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fdround)(CPUM68KState *env, FPReg *res, FPReg *val)
+{
+PREC_BEGIN(64);
+res->d = floatx80_round(val->d, &env->fp_status);
+PREC_END();
+}
+
 void HELPER(fsqrt)(CPUM68KState *env, FPReg *res, FPReg *val)
 {
 res->d = floatx80_sqrt(val->d, &env->fp_status);
@@ -188,12 +202,40 @@ void HELPER(fdsqrt)(CPUM68KState *env, FPReg *res, FPReg 
*val)
 
 void HELPER(fabs)(CPUM68KState *env, FPReg *res, FPReg *val)
 {
-res->d = floatx80_abs(val->d);
+res->d = floatx80_round(floatx80_abs(val->d), &env->fp_status);
+}
+
+void HELPER(fsabs)(CPUM68KState *env, FPReg *res, FPReg *val)
+{
+PREC_BEGIN(32);
+res->d = floatx80_round(floatx80_abs(val->d), &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fdabs)(CPUM68KState *env, FPReg *res, FPReg *val)
+{
+PREC_BEGIN(64);
+res->d = floatx80_round(floatx80_abs(val->d), &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fneg)(CPUM68KState *env, FPReg *res, FPReg *val)
+{
+res->d = floatx80_round(floatx80_chs(val->d), &env->fp_status);
 }
 
-void HELPER(fchs)(CPUM68KState *env, FPReg *res, FPReg *val)
+void HELPER(fsneg)(CPUM68KState *env, FPReg *res, FPReg *val)
 {
-res->d = floatx80_chs(val->d);
+PREC_BEGIN(32);
+res->d = floatx80_round(floatx80_chs(val->d), &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fdneg)(CPUM68KState *env, FPReg *res, FPReg *val)
+{
+PREC_BEGIN(64);
+res->d = floatx80_round(floatx80_chs(val->d), &env->fp_status);
+PREC_END();
 }
 
 void HELPER(fadd)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index f05191b..b396899 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -23,13 +23,19 @@ DEF_HELPER_2(redf32, f32, env, fp)
 DEF_HELPER_2(redf64, f64, env, fp)
 DEF_HELPER_2(reds32, s32, env, fp)
 
+DEF_HELPER_3(fsround, void, env, fp, fp)
+DEF_HELPER_3(fdround, void, env, fp, fp)
 DEF_HELPER_3(firound, void, env, fp, fp)
 DEF_HELPER_3(fitrunc, void, env, fp, fp)
 DEF_HELPER_3(fsqrt, void, env, fp, fp)
 DEF_HELPER_3(fssqrt, void, env, fp, fp)
 DEF_HELPER_3(fdsqrt, void, env, fp, fp)
 DEF_HELPER_3(fabs, void, env, fp, fp)
-DEF_HELPER_3(fchs, void, env, fp, fp)
+DEF_HELPER_3(fsabs, void, env, fp, fp)
+DEF_HELPER_3(fdabs, void, env, fp, fp)
+DEF_HELPER_3(fneg, void, env, fp, fp)
+DEF_HELPER_3(fsneg, void, env, fp, fp)
+DEF_HELPER_3(fdneg, void, env, fp, fp)
 DEF_HELPER_4(fadd, void, env, fp, fp, fp)
 DEF_HELPER_4(fsadd, void, env, fp, fp, fp)
 DEF_HELPER_4(fdadd, void, env, fp, fp, fp)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 72c45de..89ac2c7 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4595,9 +4595,15 @@ DISAS_INSN(fpu)
 }
 cpu_dest = gen_fp_ptr(REG(ext, 7));
 switch (opmode) {
-case 0: case 0x40: case 0x44: /* fmove */
+case 0: /* fmove */
 gen_fp_move(cpu_dest, cpu_src);
 break;
+case 0x40: /* fsmove */
+gen_helper_fsround(cpu_env, cpu_dest, cpu_src);
+break;
+case 0x44: /* fdmove */
+gen_helper_fdround(cpu_env, cpu_dest, cpu_src);
+break;
 case 1: /* fint */
 gen_helper_firound(cpu_env, cpu_dest, cpu_src);
 break;
@@ -4613,11 +4619,23 @@ DISAS_INSN(fpu)
 case 0x45: /* fdsqrt */
 gen_helper_fdsqrt(cpu_env, cpu_dest, cpu_src);
 break;
-case 0x18: case 0x58: case 0x5c: /* fabs */
+case 0x18: /* fabs */
 gen_helper_fabs(cpu_env, cpu_dest, cpu_src);
 break;
-case 0x1a: case 0x5a: case 0x5e: /* fneg */
-gen_helper_fchs(cpu_env, cpu_dest, cpu_src);
+case 0x58: /* fsabs */
+gen_helper_fsabs(cpu_env, cpu_dest, cpu_src);
+break;
+case 0x5c: /* fdabs */
+gen_helper_fdabs(cpu_env, cpu_dest, cpu_src);
+break;
+case 0x1a: /* fneg */
+gen_helper_fneg(cpu_env, cpu_dest, cpu_src);
+break;
+case 0x5a: /* fsneg */
+gen_helper_fsneg(cpu_env, cpu_dest, cpu_src);
+break;
+case 0x5e: /* fdneg *

[Qemu-devel] [PULL 4/7] softfloat: define floatx80_round()

[Laurent Vivier]

Add a function to round a floatx80 to the defined precision
(floatx80_rounding_precision)

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Reviewed-by: Aurelien Jarno 
Message-Id: <20170628204241.32106-5-laur...@vivier.eu>
---
 fpu/softfloat.c | 16 
 include/fpu/softfloat.h |  1 +
 2 files changed, 17 insertions(+)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7af14e2..433c5da 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -5086,6 +5086,22 @@ float128 floatx80_to_float128(floatx80 a, float_status 
*status)
 }
 
 /*
+| Rounds the extended double-precision floating-point value `a'
+| to the precision provided by floatx80_rounding_precision and returns the
+| result as an extended double-precision floating-point value.
+| The operation is performed according to the IEC/IEEE Standard for Binary
+| Floating-Point Arithmetic.
+**/
+
+floatx80 floatx80_round(floatx80 a, float_status *status)
+{
+return roundAndPackFloatx80(status->floatx80_rounding_precision,
+extractFloatx80Sign(a),
+extractFloatx80Exp(a),
+extractFloatx80Frac(a), 0, status);
+}
+
+/*
 | Rounds the extended double-precision floating-point value `a' to an integer,
 | and returns the result as an extended quadruple-precision floating-point
 | value.  The operation is performed according to the IEC/IEEE Standard for
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index f1288ef..d9689ec 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -621,6 +621,7 @@ float128 floatx80_to_float128(floatx80, float_status 
*status);
 /*
 | Software IEC/IEEE extended double-precision operations.
 **/
+floatx80 floatx80_round(floatx80 a, float_status *status);
 floatx80 floatx80_round_to_int(floatx80, float_status *status);
 floatx80 floatx80_add(floatx80, floatx80, float_status *status);
 floatx80 floatx80_sub(floatx80, floatx80, float_status *status);
-- 
2.9.4

[Qemu-devel] [PULL 7/7] target/m68k: add fmovem

[Laurent Vivier]

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Message-Id: <20170628204241.32106-8-laur...@vivier.eu>
---
 target/m68k/fpu_helper.c | 120 +++
 target/m68k/helper.h |   6 +++
 target/m68k/translate.c  |  93 
 3 files changed, 189 insertions(+), 30 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 382fbc1..bdfc537 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -22,6 +22,7 @@
 #include "cpu.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
+#include "exec/cpu_ldst.h"
 
 /* Undefined offsets may be different on various FPU.
  * On 68040 they return 0.0 (floatx80_zero)
@@ -388,3 +389,122 @@ void HELPER(fconst)(CPUM68KState *env, FPReg *val, 
uint32_t offset)
 {
 val->d = fpu_rom[offset];
 }
+
+typedef int (*float_access)(CPUM68KState *env, uint32_t addr, FPReg *fp,
+uintptr_t ra);
+
+static uint32_t fmovem_predec(CPUM68KState *env, uint32_t addr, uint32_t mask,
+   float_access access)
+{
+uintptr_t ra = GETPC();
+int i, size;
+
+for (i = 7; i >= 0; i--, mask <<= 1) {
+if (mask & 0x80) {
+size = access(env, addr, &env->fregs[i], ra);
+if ((mask & 0xff) != 0x80) {
+addr -= size;
+}
+}
+}
+
+return addr;
+}
+
+static uint32_t fmovem_postinc(CPUM68KState *env, uint32_t addr, uint32_t mask,
+   float_access access)
+{
+uintptr_t ra = GETPC();
+int i, size;
+
+for (i = 0; i < 8; i++, mask <<= 1) {
+if (mask & 0x80) {
+size = access(env, addr, &env->fregs[i], ra);
+addr += size;
+}
+}
+
+return addr;
+}
+
+static int cpu_ld_floatx80_ra(CPUM68KState *env, uint32_t addr, FPReg *fp,
+  uintptr_t ra)
+{
+uint32_t high;
+uint64_t low;
+
+high = cpu_ldl_data_ra(env, addr, ra);
+low = cpu_ldq_data_ra(env, addr + 4, ra);
+
+fp->l.upper = high >> 16;
+fp->l.lower = low;
+
+return 12;
+}
+
+static int cpu_st_floatx80_ra(CPUM68KState *env, uint32_t addr, FPReg *fp,
+   uintptr_t ra)
+{
+cpu_stl_data_ra(env, addr, fp->l.upper << 16, ra);
+cpu_stq_data_ra(env, addr + 4, fp->l.lower, ra);
+
+return 12;
+}
+
+static int cpu_ld_float64_ra(CPUM68KState *env, uint32_t addr, FPReg *fp,
+ uintptr_t ra)
+{
+uint64_t val;
+
+val = cpu_ldq_data_ra(env, addr, ra);
+fp->d = float64_to_floatx80(*(float64 *)&val, &env->fp_status);
+
+return 8;
+}
+
+static int cpu_st_float64_ra(CPUM68KState *env, uint32_t addr, FPReg *fp,
+ uintptr_t ra)
+{
+float64 val;
+
+val = floatx80_to_float64(fp->d, &env->fp_status);
+cpu_stq_data_ra(env, addr, *(uint64_t *)&val, ra);
+
+return 8;
+}
+
+uint32_t HELPER(fmovemx_st_predec)(CPUM68KState *env, uint32_t addr,
+   uint32_t mask)
+{
+return fmovem_predec(env, addr, mask, cpu_st_floatx80_ra);
+}
+
+uint32_t HELPER(fmovemx_st_postinc)(CPUM68KState *env, uint32_t addr,
+uint32_t mask)
+{
+return fmovem_postinc(env, addr, mask, cpu_st_floatx80_ra);
+}
+
+uint32_t HELPER(fmovemx_ld_postinc)(CPUM68KState *env, uint32_t addr,
+uint32_t mask)
+{
+return fmovem_postinc(env, addr, mask, cpu_ld_floatx80_ra);
+}
+
+uint32_t HELPER(fmovemd_st_predec)(CPUM68KState *env, uint32_t addr,
+   uint32_t mask)
+{
+return fmovem_predec(env, addr, mask, cpu_st_float64_ra);
+}
+
+uint32_t HELPER(fmovemd_st_postinc)(CPUM68KState *env, uint32_t addr,
+uint32_t mask)
+{
+return fmovem_postinc(env, addr, mask, cpu_st_float64_ra);
+}
+
+uint32_t HELPER(fmovemd_ld_postinc)(CPUM68KState *env, uint32_t addr,
+uint32_t mask)
+{
+return fmovem_postinc(env, addr, mask, cpu_ld_float64_ra);
+}
diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index b396899..475a1f2 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -54,6 +54,12 @@ DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp)
 DEF_HELPER_FLAGS_2(set_fpcr, TCG_CALL_NO_RWG, void, env, i32)
 DEF_HELPER_FLAGS_2(ftst, TCG_CALL_NO_RWG, void, env, fp)
 DEF_HELPER_3(fconst, void, env, fp, i32)
+DEF_HELPER_3(fmovemx_st_predec, i32, env, i32, i32)
+DEF_HELPER_3(fmovemx_st_postinc, i32, env, i32, i32)
+DEF_HELPER_3(fmovemx_ld_postinc, i32, env, i32, i32)
+DEF_HELPER_3(fmovemd_st_predec, i32, env, i32, i32)
+DEF_HELPER_3(fmovemd_st_postinc, i32, env, i32, i32)
+DEF_HELPER_3(fmovemd_ld_postinc, i32, env, i32, i32)
 
 DEF_HELPER_3(mac_move, void, env, i32, i32)
 DEF_HELPER_3(macmulf, i64, env, i32, i32)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
i

[Qemu-devel] [PULL 3/7] target/m68k: add explicit single and double precision operations

[Laurent Vivier]

Add fssqrt, fdsqrt, fsadd, fdadd, fssub, fdsub, fsmul, fdmul,
fsdiv, fddiv.

The precision is managed using set_floatx80_rounding_precision().

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Message-Id: <20170628204241.32106-4-laur...@vivier.eu>
---
 target/m68k/fpu_helper.c | 80 
 target/m68k/helper.h | 10 ++
 target/m68k/translate.c  | 40 +---
 3 files changed, 125 insertions(+), 5 deletions(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 4c14a1f..3b53554 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -157,11 +157,35 @@ void HELPER(set_fpcr)(CPUM68KState *env, uint32_t val)
 cpu_m68k_set_fpcr(env, val);
 }
 
+#define PREC_BEGIN(prec)\
+do {\
+int old;\
+old = get_floatx80_rounding_precision(&env->fp_status); \
+set_floatx80_rounding_precision(prec, &env->fp_status)  \
+
+#define PREC_END()  \
+set_floatx80_rounding_precision(old, &env->fp_status);  \
+} while (0)
+
 void HELPER(fsqrt)(CPUM68KState *env, FPReg *res, FPReg *val)
 {
 res->d = floatx80_sqrt(val->d, &env->fp_status);
 }
 
+void HELPER(fssqrt)(CPUM68KState *env, FPReg *res, FPReg *val)
+{
+PREC_BEGIN(32);
+res->d = floatx80_sqrt(val->d, &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fdsqrt)(CPUM68KState *env, FPReg *res, FPReg *val)
+{
+PREC_BEGIN(64);
+res->d = floatx80_sqrt(val->d, &env->fp_status);
+PREC_END();
+}
+
 void HELPER(fabs)(CPUM68KState *env, FPReg *res, FPReg *val)
 {
 res->d = floatx80_abs(val->d);
@@ -177,21 +201,77 @@ void HELPER(fadd)(CPUM68KState *env, FPReg *res, FPReg 
*val0, FPReg *val1)
 res->d = floatx80_add(val0->d, val1->d, &env->fp_status);
 }
 
+void HELPER(fsadd)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+PREC_BEGIN(32);
+res->d = floatx80_add(val0->d, val1->d, &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fdadd)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+PREC_BEGIN(64);
+res->d = floatx80_add(val0->d, val1->d, &env->fp_status);
+PREC_END();
+}
+
 void HELPER(fsub)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 {
 res->d = floatx80_sub(val1->d, val0->d, &env->fp_status);
 }
 
+void HELPER(fssub)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+PREC_BEGIN(32);
+res->d = floatx80_sub(val1->d, val0->d, &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fdsub)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+PREC_BEGIN(64);
+res->d = floatx80_sub(val1->d, val0->d, &env->fp_status);
+PREC_END();
+}
+
 void HELPER(fmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 {
 res->d = floatx80_mul(val0->d, val1->d, &env->fp_status);
 }
 
+void HELPER(fsmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+PREC_BEGIN(32);
+res->d = floatx80_mul(val0->d, val1->d, &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fdmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+PREC_BEGIN(64);
+res->d = floatx80_mul(val0->d, val1->d, &env->fp_status);
+PREC_END();
+}
+
 void HELPER(fdiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 {
 res->d = floatx80_div(val1->d, val0->d, &env->fp_status);
 }
 
+void HELPER(fsdiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+PREC_BEGIN(32);
+res->d = floatx80_div(val1->d, val0->d, &env->fp_status);
+PREC_END();
+}
+
+void HELPER(fddiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+PREC_BEGIN(64);
+res->d = floatx80_div(val1->d, val0->d, &env->fp_status);
+PREC_END();
+}
+
 static int float_comp_to_cc(int float_compare)
 {
 switch (float_compare) {
diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index d6e80e4..0c7f06f 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -26,12 +26,22 @@ DEF_HELPER_2(reds32, s32, env, fp)
 DEF_HELPER_3(firound, void, env, fp, fp)
 DEF_HELPER_3(fitrunc, void, env, fp, fp)
 DEF_HELPER_3(fsqrt, void, env, fp, fp)
+DEF_HELPER_3(fssqrt, void, env, fp, fp)
+DEF_HELPER_3(fdsqrt, void, env, fp, fp)
 DEF_HELPER_3(fabs, void, env, fp, fp)
 DEF_HELPER_3(fchs, void, env, fp, fp)
 DEF_HELPER_4(fadd, void, env, fp, fp, fp)
+DEF_HELPER_4(fsadd, void, env, fp, fp, fp)
+DEF_HELPER_4(fdadd, void, env, fp, fp, fp)
 DEF_HELPER_4(fsub, void, env, fp, fp, fp)
+DEF_HELPER_4(fssub, void, env, fp, fp, fp)
+DEF_HELPER_4(fdsub, void, env, fp, fp, fp)
 DEF_HELPER_4(fmul, void, env, fp, fp, fp)
+DEF_HELPER_4(fsmul, void, env, fp, fp, fp)
+DEF_HELPER_4(fdmul, void, env, fp, fp, fp)
 DEF_HELPER_4(fdiv, void, env, fp, fp, fp)
+DEF_HELPER_4(fsdiv, void, env, fp, fp, fp)
+DEF_HELPER_4(fddiv, void, env, fp, fp, fp)
 DEF_HELP

[Qemu-devel] [PULL 5/7] target/m68k: add fsglmul and fsgldiv

[Laurent Vivier]

fsglmul and fsgldiv truncate data to single precision before computing
results.

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Message-Id: <20170628204241.32106-6-laur...@vivier.eu>
---
 target/m68k/fpu_helper.c | 28 
 target/m68k/helper.h |  2 ++
 target/m68k/translate.c  |  6 ++
 3 files changed, 36 insertions(+)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index 3b53554..600ae8a 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -253,6 +253,20 @@ void HELPER(fdmul)(CPUM68KState *env, FPReg *res, FPReg 
*val0, FPReg *val1)
 PREC_END();
 }
 
+void HELPER(fsglmul)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+int rounding_mode = get_float_rounding_mode(&env->fp_status);
+floatx80 a, b;
+
+PREC_BEGIN(32);
+set_float_rounding_mode(float_round_to_zero, &env->fp_status);
+a = floatx80_round(val0->d, &env->fp_status);
+b = floatx80_round(val1->d, &env->fp_status);
+set_float_rounding_mode(rounding_mode, &env->fp_status);
+res->d = floatx80_mul(a, b, &env->fp_status);
+PREC_END();
+}
+
 void HELPER(fdiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
 {
 res->d = floatx80_div(val1->d, val0->d, &env->fp_status);
@@ -272,6 +286,20 @@ void HELPER(fddiv)(CPUM68KState *env, FPReg *res, FPReg 
*val0, FPReg *val1)
 PREC_END();
 }
 
+void HELPER(fsgldiv)(CPUM68KState *env, FPReg *res, FPReg *val0, FPReg *val1)
+{
+int rounding_mode = get_float_rounding_mode(&env->fp_status);
+floatx80 a, b;
+
+PREC_BEGIN(32);
+set_float_rounding_mode(float_round_to_zero, &env->fp_status);
+a = floatx80_round(val1->d, &env->fp_status);
+b = floatx80_round(val0->d, &env->fp_status);
+set_float_rounding_mode(rounding_mode, &env->fp_status);
+res->d = floatx80_div(a, b, &env->fp_status);
+PREC_END();
+}
+
 static int float_comp_to_cc(int float_compare)
 {
 switch (float_compare) {
diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index 0c7f06f..f05191b 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -39,9 +39,11 @@ DEF_HELPER_4(fdsub, void, env, fp, fp, fp)
 DEF_HELPER_4(fmul, void, env, fp, fp, fp)
 DEF_HELPER_4(fsmul, void, env, fp, fp, fp)
 DEF_HELPER_4(fdmul, void, env, fp, fp, fp)
+DEF_HELPER_4(fsglmul, void, env, fp, fp, fp)
 DEF_HELPER_4(fdiv, void, env, fp, fp, fp)
 DEF_HELPER_4(fsdiv, void, env, fp, fp, fp)
 DEF_HELPER_4(fddiv, void, env, fp, fp, fp)
+DEF_HELPER_4(fsgldiv, void, env, fp, fp, fp)
 DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp)
 DEF_HELPER_FLAGS_2(set_fpcr, TCG_CALL_NO_RWG, void, env, i32)
 DEF_HELPER_FLAGS_2(ftst, TCG_CALL_NO_RWG, void, env, fp)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 618abf6..72c45de 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4646,6 +4646,12 @@ DISAS_INSN(fpu)
 case 0x67: /* fdmul */
 gen_helper_fdmul(cpu_env, cpu_dest, cpu_src, cpu_dest);
 break;
+case 0x24: /* fsgldiv */
+gen_helper_fsgldiv(cpu_env, cpu_dest, cpu_src, cpu_dest);
+break;
+case 0x27: /* fsglmul */
+gen_helper_fsglmul(cpu_env, cpu_dest, cpu_src, cpu_dest);
+break;
 case 0x28: /* fsub */
 gen_helper_fsub(cpu_env, cpu_dest, cpu_src, cpu_dest);
 break;
-- 
2.9.4

[Qemu-devel] [PULL 2/7] target/m68k: add fmovecr

[Laurent Vivier]

fmovecr moves a floating point constant from the
FPU ROM to a floating point register.

Signed-off-by: Laurent Vivier 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20170628204241.32106-3-laur...@vivier.eu>
---
 target/m68k/fpu_helper.c | 34 ++
 target/m68k/helper.h |  1 +
 target/m68k/translate.c  | 13 -
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c
index a9e17f5..4c14a1f 100644
--- a/target/m68k/fpu_helper.c
+++ b/target/m68k/fpu_helper.c
@@ -23,6 +23,35 @@
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
 
+/* Undefined offsets may be different on various FPU.
+ * On 68040 they return 0.0 (floatx80_zero)
+ */
+
+static const floatx80 fpu_rom[128] = {
+[0x00] = floatx80_pi,   /* Pi */
+[0x0b] = make_floatx80(0x3ffd, 0x9a209a84fbcff798ULL),  /* Log10(2) */
+[0x0c] = make_floatx80(0x4000, 0xadf85458a2bb4a9aULL),  /* e*/
+[0x0d] = make_floatx80(0x3fff, 0xb8aa3b295c17f0bcULL),  /* Log2(e)  */
+[0x0e] = make_floatx80(0x3ffd, 0xde5bd8a937287195ULL),  /* Log10(e) */
+[0x0f] = floatx80_zero, /* Zero */
+[0x30] = floatx80_ln2,  /* ln(2)*/
+[0x31] = make_floatx80(0x4000, 0x935d8dddaaa8ac17ULL),  /* ln(10)   */
+[0x32] = floatx80_one,  /* 10^0 */
+[0x33] = make_floatx80(0x4002, 0xa000ULL),  /* 10^1 */
+[0x34] = make_floatx80(0x4005, 0xc800ULL),  /* 10^2 */
+[0x35] = make_floatx80(0x400c, 0x9c40ULL),  /* 10^4 */
+[0x36] = make_floatx80(0x4019, 0xbebc2000ULL),  /* 10^8 */
+[0x37] = make_floatx80(0x4034, 0x8e1bc9bf0400ULL),  /* 10^16*/
+[0x38] = make_floatx80(0x4069, 0x9dc5ada82b70b59eULL),  /* 10^32*/
+[0x39] = make_floatx80(0x40d3, 0xc2781f49ffcfa6d5ULL),  /* 10^64*/
+[0x3a] = make_floatx80(0x41a8, 0x93ba47c980e98ce0ULL),  /* 10^128   */
+[0x3b] = make_floatx80(0x4351, 0xaa7eebfb9df9de8eULL),  /* 10^256   */
+[0x3c] = make_floatx80(0x46a3, 0xe319a0aea60e91c7ULL),  /* 10^512   */
+[0x3d] = make_floatx80(0x4d48, 0xc976758681750c17ULL),  /* 10^1024  */
+[0x3e] = make_floatx80(0x5a92, 0x9e8b3b5dc53d5de5ULL),  /* 10^2048  */
+[0x3f] = make_floatx80(0x7525, 0xc46052028a20979bULL),  /* 10^4096  */
+};
+
 int32_t HELPER(reds32)(CPUM68KState *env, FPReg *val)
 {
 return floatx80_to_int32(val->d, &env->fp_status);
@@ -204,3 +233,8 @@ void HELPER(ftst)(CPUM68KState *env, FPReg *val)
 }
 env->fpsr = (env->fpsr & ~FPSR_CC_MASK) | cc;
 }
+
+void HELPER(fconst)(CPUM68KState *env, FPReg *val, uint32_t offset)
+{
+val->d = fpu_rom[offset];
+}
diff --git a/target/m68k/helper.h b/target/m68k/helper.h
index 98cbf18..d6e80e4 100644
--- a/target/m68k/helper.h
+++ b/target/m68k/helper.h
@@ -35,6 +35,7 @@ DEF_HELPER_4(fdiv, void, env, fp, fp, fp)
 DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp)
 DEF_HELPER_FLAGS_2(set_fpcr, TCG_CALL_NO_RWG, void, env, i32)
 DEF_HELPER_FLAGS_2(ftst, TCG_CALL_NO_RWG, void, env, fp)
+DEF_HELPER_3(fconst, void, env, fp, i32)
 
 DEF_HELPER_3(mac_move, void, env, i32, i32)
 DEF_HELPER_3(macmulf, i64, env, i32, i32)
diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 5f4bedc..5b93d3f 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -4518,10 +4518,21 @@ DISAS_INSN(fpu)
 ext = read_im16(env, s);
 opmode = ext & 0x7f;
 switch ((ext >> 13) & 7) {
-case 0: case 2:
+case 0:
 break;
 case 1:
 goto undef;
+case 2:
+if (insn == 0xf200 && (ext & 0xfc00) == 0x5c00) {
+/* fmovecr */
+TCGv rom_offset = tcg_const_i32(opmode);
+cpu_dest = gen_fp_ptr(REG(ext, 7));
+gen_helper_fconst(cpu_env, cpu_dest, rom_offset);
+tcg_temp_free_ptr(cpu_dest);
+tcg_temp_free(rom_offset);
+return;
+}
+break;
 case 3: /* fmove out */
 cpu_src = gen_fp_ptr(REG(ext, 7));
 opsize = ext_opsize(ext, 10);
-- 
2.9.4

Re: [Qemu-devel] [PATCH v6 04/10] migration: let MigrationState be a qdev

[Max Reitz]

On 2017-06-30 09:11, Peter Xu wrote:
> On Fri, Jun 30, 2017 at 11:03:21AM +0800, Peter Xu wrote:
>> On Fri, Jun 30, 2017 at 04:18:56AM +0200, Max Reitz wrote:
>>> On 2017-06-27 06:10, Peter Xu wrote:
 Let the old man "MigrationState" join the object family. Direct benefit
 is that we can start to use all the property features derived from
 current QDev, like: HW_COMPAT_* bits, command line setup for migration
 parameters (so will never need to set them up each time using HMP/QMP,
 this is really, really attractive for test writters), etc.

 I see no reason to disallow this happen yet. So let's start from this
 one, to see whether it would be anything good.

 Now we init the MigrationState struct statically in main() to make sure
 it's initialized after global properties are applied, since we'll use
 them during creation of the object.

 No functional change at all.
>>>
>>> Evidently not quite right because this breaks iotest 055.
>>>
>>> Condensed test case:
>>>
>>> $ ./qemu-img create -f vmdk -o subformat=streamOptimized foo.vmdk 64M
>>> Formatting 'foo.vmdk', fmt=vmdk size=67108864 compat6=off
>>> hwversion=undefined subformat=streamOptimized
>>> $ x86_64-softmmu/qemu-system-x86_64 -drive if=none,file=foo.vmdk
>>> qemu-system-x86_64: ./migration/migration.c:114: migrate_get_current:
>>> Assertion `current_migration' failed.
>>> [1]15453 abort (core dumped)  x86_64-softmmu/qemu-system-x86_64
>>> -drive if=none,file=foo.vmdk
>>>
>>> (It just worked before this patch.)
>>
>> Sorry. Will have a look.
> 
> Hello, Max,
> 
> The assertion is caused by migrate_add_blocker() called before
> initialization of migration object. I'll fix it.

Thanks!

> But even with a fix (so I can pass 055 now), I still cannot pass some
> of the other tests. Errors I got:
> 
>   https://pastebin.com/ACqbXAYd
> 
> I am not familiar with iotests. Is above usual? Looks like it still
> includes 3 failures, and some output mismatch.

Well, not usual. But 068 just is broken on master currently (Stefan has
sent "virtio: use ioeventfd in TCG and qtest mode" to fix it, and it's
part of his latest pull request). The failure in test 087 is because you
don't have aio=native enabled in your build, as the message says. :-)

I'm not sure about 118. Maybe the os.chmod() doesn't work as intended on
your machine...? Because it tries to open a read-only image as
read/write and wants to see it fail (which it doesn't in your case).

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2] xen-platform: Cleanup network infrastructure when emulated NICs are unplugged

[Ross Lagerwall]

When the guest unplugs the emulated NICs, cleanup the peer for each NIC
as it is not needed anymore. Most importantly, this allows the tap
interfaces which QEMU holds open to be closed and removed.

Signed-off-by: Ross Lagerwall 
---

In v2: Don't call nic_cleanup(), just remove the peer of the NIC which
will cleanup up the tap devices. This means that QEMU doesn't segv
when shutting down.

 hw/i386/xen/xen_platform.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/i386/xen/xen_platform.c b/hw/i386/xen/xen_platform.c
index 1419fc9..f231558 100644
--- a/hw/i386/xen/xen_platform.c
+++ b/hw/i386/xen/xen_platform.c
@@ -102,8 +102,19 @@ static void unplug_nic(PCIBus *b, PCIDevice *d, void *o)
 }
 }
 
+/* Remove the peer of the NIC device. Normally, this would be a tap device. */
+static void del_nic_peer(NICState *nic, void *opaque)
+{
+NetClientState *nc;
+
+nc = qemu_get_queue(nic);
+if (nc->peer)
+qemu_del_net_client(nc->peer);
+}
+
 static void pci_unplug_nics(PCIBus *bus)
 {
+qemu_foreach_nic(del_nic_peer, NULL);
 pci_for_each_device(bus, 0, unplug_nic, NULL);
 }
 
-- 
2.9.4

Re: [Qemu-devel] [Qemu-block] [RFC] QMP design: Fixing query-block and friends

[Alberto Garcia]

On Tue 27 Jun 2017 06:31:45 PM CEST, Kevin Wolf wrote:
> * Speaking of recursion: ImageInfo recursively includes information
>   about all images in the backing chain. This is what makes the output
>   of query-named-block-nodes so redundant. It is also inconsistent
>   because the runtime information (BlockInfo/BlockDeviceInfo) isn't
>   recursive.

I've also been told of cases where a query-block command on a VM with a
very large amount of images in all backing chains would slow down the
guest because of this recursion.

I haven't been able to reproduce this problem myself, but being able to
see only the devices seen by the guest (as opposed to the complete
graph) seems like a good idea.

Berto

[Qemu-devel] BIT_WORD(start >> TARGET_PAGE_BITS)

[ali saeedi]

Hello
what does the following code do?
'unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS)' ?
thanks a lot

Re: [Qemu-devel] [PATCH v6 04/10] migration: let MigrationState be a qdev

[Eric Blake]

On 06/30/2017 07:33 AM, Max Reitz wrote:

>> The assertion is caused by migrate_add_blocker() called before
>> initialization of migration object. I'll fix it.
> 
> Thanks!
> 
>> But even with a fix (so I can pass 055 now), I still cannot pass some
>> of the other tests. Errors I got:
>>
>>   https://pastebin.com/ACqbXAYd
>>
>> I am not familiar with iotests. Is above usual? Looks like it still
>> includes 3 failures, and some output mismatch.
> 
> Well, not usual. But 068 just is broken on master currently (Stefan has
> sent "virtio: use ioeventfd in TCG and qtest mode" to fix it, and it's
> part of his latest pull request). The failure in test 087 is because you
> don't have aio=native enabled in your build, as the message says. :-)

We could obviously patch 087 to gracefully skip instead of fail when the
build doesn't support running it.

> 
> I'm not sure about 118. Maybe the os.chmod() doesn't work as intended on
> your machine...? Because it tries to open a read-only image as
> read/write and wants to see it fail (which it doesn't in your case).

Maybe a run-as-root issue, where root can write to the file in spite of
permissions?  Ideally, I'm reluctant to run testsuites as root without
good reason (or at least a good sandbox), for fear that a bug in the
testsuite will hose my system.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] spapr: make spapr_populate_hotplug_cpu_dt() static

[Greg Kurz]

Since commit ff9006ddbfd1 ("spapr: move spapr_core_[foo]plug() callbacks
close to machine code in spapr.c"), this function doesn't need to be extern
anymore.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr.c |4 ++--
 include/hw/ppc/spapr.h |2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0ee9fac50bd4..65d8ad2f4966 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2863,8 +2863,8 @@ out:
 error_propagate(errp, local_err);
 }
 
-void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
-sPAPRMachineState *spapr)
+static void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
+   sPAPRMachineState *spapr)
 {
 PowerPCCPU *cpu = POWERPC_CPU(cs);
 DeviceClass *dc = DEVICE_GET_CLASS(cs);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index a66bbac35242..12bf9697990e 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -640,8 +640,6 @@ void 
spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
 void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
uint32_t count, uint32_t index);
 void spapr_cpu_parse_features(sPAPRMachineState *spapr);
-void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
-sPAPRMachineState *spapr);
 
 /* CPU and LMB DRC release callbacks. */
 void spapr_core_release(DeviceState *dev);

Re: [Qemu-devel] BIT_WORD(start >> TARGET_PAGE_BITS)

[Eric Blake]

On 06/30/2017 08:02 AM, ali saeedi wrote:
> Hello
> what does the following code do?
> 'unsigned long page = BIT_WORD(start >> TARGET_PAGE_BITS)' ?

I've noticed you've been asking a lot of questions (each as a new
thread, rather than replying to previous answers), that seem like you
are not trying very hard to read the code and find an answer for
yourself.  Rather than just answer you, I'm going to try to teach you
how to read the source and answer the question yourself.  You may also
get better answers to your future questions if you ask in the form of
"this code is confusing me, I think it means this, based on what I read
at xyz, but would like confirmation or correction" (showing WHAT you
have already researched) rather than just "what does this code do" (and
making it feel like you are off-loading the research work onto others).

First, figure out what BIT_WORD does:

$ git grep 'define BIT_WORD'

That should have only one hit, in include/qemu/bitops.h.  Reading it in
context doesn't have any more comments, but it looks like it is
computing the number of bits that are available in a word, and looks
like it is defining a word to be the type most efficiently operated on
for the current ABI (a long is 32 bits on a 32-bit OS, and 64 bits on a
64-bit OS).

It also looks like you are scaling a start address by the number of bits
in a target page.

So it probably means you are computing the index for which page 'start'
occurs on (depending on values, it might mean that 'start == 0x0' is
page 0, 'start == 0x1' is page 1, and so on), where start is
initially in the form of the number of bits per page.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] s390: return unavailable features via query-cpu-definitions

[Viktor Mihajlovski]

The response for query-cpu-definitions didn't include the
unavailable-features field, which is used by libvirt to figure
out whether a certain cpu model is usable on the host.

The unavailable features are now computed by obtaining the host CPU
model and comparing its feature bitmap with the feature bitmaps of
the known CPU models.

I.e. the output of virsh domcapabilities would change from
 ...
 
  z10EC-base
  z9EC-base
  z196.2-base
  z900-base
  z990
 ...
to
 ...
 
  z10EC-base
  z9EC-base
  z196.2-base
  z900-base
  z990
 ...

Signed-off-by: Viktor Mihajlovski 
---
 target/s390x/cpu_models.c | 51 ++-
 1 file changed, 46 insertions(+), 5 deletions(-)

diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 63903c2..dc3371f 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -283,14 +283,43 @@ void s390_cpu_list(FILE *f, fprintf_function print)
 }
 }
 
+static S390CPUModel *get_max_cpu_model(Error **errp);
+
 #ifndef CONFIG_USER_ONLY
+static void list_add_feat(const char *name, void *opaque);
+
+static void check_unavailable_features(const S390CPUModel *max_model,
+   const S390CPUModel *model,
+   strList **unavailable)
+{
+S390FeatBitmap missing;
+
+/* detect missing features if any to properly report them */
+bitmap_andnot(missing, model->features, max_model->features,
+  S390_FEAT_MAX);
+if (!bitmap_empty(missing, S390_FEAT_MAX)) {
+s390_feat_bitmap_to_ascii(missing,
+  unavailable,
+  list_add_feat);
+}
+}
+
+struct CpuDefinitionInfoListData {
+CpuDefinitionInfoList *list;
+Error **errp;
+};
+
 static void create_cpu_model_list(ObjectClass *klass, void *opaque)
 {
-CpuDefinitionInfoList **cpu_list = opaque;
+struct CpuDefinitionInfoListData *cpu_list_data = opaque;
+CpuDefinitionInfoList **cpu_list = &cpu_list_data->list;
 CpuDefinitionInfoList *entry;
 CpuDefinitionInfo *info;
 char *name = g_strdup(object_class_get_name(klass));
 S390CPUClass *scc = S390_CPU_CLASS(klass);
+Object *obj;
+S390CPU *sc;
+S390CPUModel *scm;
 
 /* strip off the -s390-cpu */
 g_strrstr(name, "-" TYPE_S390_CPU)[0] = 0;
@@ -300,21 +329,33 @@ static void create_cpu_model_list(ObjectClass *klass, 
void *opaque)
 info->migration_safe = scc->is_migration_safe;
 info->q_static = scc->is_static;
 info->q_typename = g_strdup(object_class_get_name(klass));
-
+/* check for unavailable features */
+obj = object_new(object_class_get_name(klass));
+sc = S390_CPU(obj);
+scm = get_max_cpu_model(cpu_list_data->errp);
+if (scm && sc->model) {
+info->has_unavailable_features = true;
+check_unavailable_features(scm, sc->model, 
&info->unavailable_features);
+}
 
 entry = g_malloc0(sizeof(*entry));
 entry->value = info;
 entry->next = *cpu_list;
 *cpu_list = entry;
+object_unref(obj);
 }
 
 CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
 {
-CpuDefinitionInfoList *list = NULL;
+struct CpuDefinitionInfoListData list_data = {
+.list = NULL,
+.errp = errp,
+};
 
-object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false, &list);
+object_class_foreach(create_cpu_model_list, TYPE_S390_CPU, false,
+ &list_data);
 
-return list;
+return list_data.list;
 }
 
 static void cpu_model_from_info(S390CPUModel *model, const CpuModelInfo *info,
-- 
1.9.1

Re: [Qemu-devel] [PATCH v6 2/6] qmp: Create IOThrottle structure

[Alberto Garcia]

On Thu 29 Jun 2017 05:10:52 PM CEST, Pradeep Jagadeesh wrote:
> This patch enables qmp interfaces for the fsdev
> devices. This provides two interfaces one 
> for querying info of all the fsdev devices. The second one
> to set the IO limits for the required fsdev device.
>
> Signed-off-by: Pradeep Jagadeesh 
> Reviewed-by: Greg Kurz 
> Reviewed-by: Eric Blake 

Reviewed-by: Alberto Garcia 

Berto

[Qemu-devel] [PATCH v4] hmp, qmp: introduce memory-size-summary commands

[Vadim Galitsyn]

Hi Guys,

This thread is a continuation of discussion started at 
http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg05972.html.

Vadim

[Qemu-devel] [PATCH v4] hmp, qmp: introduce memory-size-summary commands

[Vadim Galitsyn]

Commands above provide the following memory information in bytes:

  * base-memory - amount of unremovable memory specified
with '-m' option at the start of the QEMU process.

  * hotpluggable-memory - amount of memory that was hot-plugged.
If target does not have CONFIG_MEM_HOTPLUG enabled, no
value is reported.

  * balloon-actual-memory - size of the memory that remains
available to the guest after ballooning, as reported by the
guest. If the guest has not reported its memory, this value
equals to @base-memory + @hot-plug-memory. If ballooning
is not enabled, no value is reported.

NOTE:

Parameter @balloon-actual-memory reports the same as
"info balloon" command when ballooning is enabled. The idea
to have it in scope of this command(s) comes from
https://lists.gnu.org/archive/html/qemu-devel/2012-07/msg01472.html.

Signed-off-by: Vasilis Liaskovitis 
Signed-off-by: Mohammed Gamal 
Signed-off-by: Eduardo Otubo 
Signed-off-by: Vadim Galitsyn 
Reviewed-by: Eugene Crosser 
Cc: Dr. David Alan Gilbert 
Cc: Markus Armbruster 
Cc: Igor Mammedov 
Cc: Eric Blake 
Cc: qemu-devel@nongnu.org
---

v4:
 * Commands "info memory" and "query-memory" were renamed
   to "info memory-size-summary" and "query-memory-size-summary"
   correspondingly.
 * Descriptions for both commands as well as MemoryInfo structure
   fields were updated/renamed according to
   http://lists.nongnu.org/archive/html/qemu-devel/2017-06/msg05972.html.
 * In MemoryInfo structure following fields are now optional:
   hotpluggable-memory and balloon-actual-memory.
 * Field "hotpluggable-memory" now not displayed in HMP if target
   has no CONFIG_MEM_HOTPLUG enabled.
 * Field "balloon-actual-memory" now not displayed in HMP if
   ballooning not enabled.
 * qapi_free_MemoryInfo() used in order to free corresponding memory
   instead of g_free().
 * #ifdef CONFIG_MEM_HOTPLUG was removed and replaced with stubs/ approach.
   get_exiting_hotpluggable_memory_size() function was introduced in
   hw/mem/pc-dimm.c (available for all targets which have CONFIG_MEM_HOTPLUG
   enabled). For other targets, there is a stub in stubs/qmp_pc_dimm.c.
   In addition, stubs/qmp_pc_dimm_device_list.c was renamed to
   stubs/qmp_pc_dimm.c in order to reflect actual source file content.
 * Commit message was updated in order to reflect what was changed.

v3:
 * Use PRIu64 instead of 'lu' when printing results via HMP.
 * Report zero hot-plugged memory instead of reporting error
   when target architecture has no CONFIG_MEM_HOTPLUG enabled.

v2:
 * Fixed build for targets which do not have CONFIG_MEM_HOTPLUG
   enabled.

 hmp-commands-info.hx   | 17 
 hmp.c  | 23 
 hmp.h  |  1 +
 hw/mem/pc-dimm.c   |  6 +
 include/hw/mem/pc-dimm.h   |  1 +
 qapi-schema.json   | 28 +++
 qmp.c  | 31 ++
 stubs/Makefile.objs|  2 +-
 stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} |  5 
 9 files changed, 113 insertions(+), 1 deletion(-)
 rename stubs/{qmp_pc_dimm_device_list.c => qmp_pc_dimm.c} (60%)

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index ba98e581ab..a535960157 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -829,6 +829,23 @@ ETEXI
 .cmd = hmp_info_vm_generation_id,
 },
 
+STEXI
+@item info memory-size-summary
+@findex memory-size-summary
+Display the amount of initially allocated, hot-plugged (if enabled)
+and ballooned (if enabled) memory in bytes.
+ETEXI
+
+{
+.name   = "memory-size-summary",
+.args_type  = "",
+.params = "",
+.help   = "show the amount of initially allocated, "
+  "hot-plugged (if enabled) and ballooned (if enabled) "
+  "memory in bytes.",
+.cmd= hmp_info_memory_size_summary,
+},
+
 STEXI
 @end table
 ETEXI
diff --git a/hmp.c b/hmp.c
index dee40284c1..15f632481c 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2828,3 +2828,26 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict 
*qdict)
 hmp_handle_error(mon, &err);
 qapi_free_GuidInfo(info);
 }
+
+void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict)
+{
+Error *err = NULL;
+MemoryInfo *info = qmp_query_memory_size_summary(&err);
+if (info) {
+monitor_printf(mon, "base-memory: %" PRIu64 "\n",
+   info->base_memory);
+
+if (info->has_hotpluggable_memory) {
+monitor_printf(mon, "hotpluggable-memory: %" PRIu64 "\n",
+   info->hotpluggable_memory);
+}
+
+if (info->has_balloon_actual_memory) {
+monitor_printf(mon, "balloon-actual-memory: %" PRIu64 "\n",

Re: [Qemu-devel] [PATCH 3/6] throttle: move out function to reuse the code

[Alberto Garcia]

On Thu 29 Jun 2017 05:10:53 PM CEST, Pradeep Jagadeesh wrote:
> This patch move out the throttle code to util/throttle.c to maximize
> the reusability of the code.The same code is also used by fsdev.
>
> Signed-off-by: Pradeep Jagadeesh 

> index 659a410..ac4f221 100644
> --- a/include/qemu/throttle-options.h
> +++ b/include/qemu/throttle-options.h
> @@ -10,6 +10,8 @@
>  #ifndef THROTTLE_OPTIONS_H
>  #define THROTTLE_OPTIONS_H
>  
> +#include "qmp-commands.h"
> +

Shouldn't this be qapi-types.h instead ?

Otherwise the patch looks fine.

Berto

[Qemu-devel] [PATCH v1 0/2] arm: Extend PAR format determination

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" 

Hi,

These are a couple of patches that implement more details of the
32 vs 64bit PAR format determination for AT operations.
This is due to issues we ran into when running Xen.

Best regards,
Edgar

Edgar E. Iglesias (2):
  target-arm: Move the regime_xxx helpers
  target-arm: Extend PAR format determination

 target/arm/helper.c | 434 
 1 file changed, 231 insertions(+), 203 deletions(-)

-- 
2.7.4