[Qemu-devel] [PATCH qemu v6] spapr: Support NVIDIA V100 GPU with NVLink2

2019-03-11 Thread Alexey Kardashevskiy
NVIDIA V100 GPUs have on-board RAM which is mapped into the host memory
space and accessible as normal RAM via an NVLink bus. The VFIO-PCI driver
implements special regions for such GPUs and emulates an NVLink bridge.
NVLink2-enabled POWER9 CPUs also provide address translation services
which includes an ATS shootdown (ATSD) register exported via the NVLink
bridge device.

This adds a quirk to VFIO to map the GPU memory and create an MR;
the new MR is stored in a PCI device as a QOM link. The sPAPR PCI uses
this to get the MR and map it to the system address space.
Another quirk does the same for ATSD.

This adds additional steps to sPAPR PHB setup:

1. Search for specific GPUs and NPUs, collect findings in
sPAPRPHBState::nvgpus, manage system address space mappings;

2. Add device-specific properties such as "ibm,npu", "ibm,gpu",
"memory-block", "link-speed" to advertise the NVLink2 function to
the guest;

3. Add "mmio-atsd" to vPHB to advertise the ATSD capability;

4. Add new memory blocks (with extra "linux,memory-usable" to prevent
the guest OS from accessing the new memory until it is onlined) and
npuphb# nodes representing an NPU unit for every vPHB as the GPU driver
uses it for link discovery.

This allocates space for GPU RAM and ATSD like we do for MMIOs by
adding 2 new parameters to the phb_placement() hook. Older machine types
set these to zero.

This puts new memory nodes in a separate NUMA node to as the GPU RAM
needs to be configured equally distant from any other node in the system.
Unlike the host setup which assigns numa ids from 255 downwards, this
adds new NUMA nodes after the user configures nodes or from 1 if none
were configured.

This adds requirement similar to EEH - one IOMMU group per vPHB.
The reason for this is that ATSD registers belong to a physical NPU
so they cannot invalidate translations on GPUs attached to another NPU.
It is guaranteed by the host platform as it does not mix NVLink bridges
or GPUs from different NPU in the same IOMMU group. If more than one
IOMMU group is detected on a vPHB, this disables ATSD support for that
vPHB and prints a warning.

Signed-off-by: Alexey Kardashevskiy 
[aw: for vfio portions]
Acked-by: Alex Williamson 
---

This is based on David's ppc-for-4.0 and acked "vfio_info_cap public" from
https://patchwork.ozlabs.org/patch/1052645/

Changes:
v6:
* changed error handling in spapr-nvlink code
* fixed mmap error checking from NULL to MAP_FAILED
* changed NUMA node ids and changed the commit log about it

v5:
* converted MRs to VFIOQuirk - this fixed leaks

v4:
* fixed ATSD placement
* fixed spapr_phb_unrealize() to do nvgpu cleanup
* replaced warn_report() with Error*

v3:
* moved GPU RAM above PCI MMIO limit
* renamed QOM property to nvlink2-tgt
* moved nvlink2 code to its own file

---

The example command line for redbud system:

pbuild/qemu-aiku1804le-ppc64/ppc64-softmmu/qemu-system-ppc64 \
-nodefaults \
-chardev stdio,id=STDIO0,signal=off,mux=on \
-device spapr-vty,id=svty0,reg=0x71000110,chardev=STDIO0 \
-mon id=MON0,chardev=STDIO0,mode=readline -nographic -vga none \
-enable-kvm -m 384G \
-chardev socket,id=SOCKET0,server,nowait,host=localhost,port=4 \
-mon chardev=SOCKET0,mode=control \
-smp 80,sockets=1,threads=4 \
-netdev "tap,id=TAP0,helper=/home/aik/qemu-bridge-helper --br=br0" \
-device "virtio-net-pci,id=vnet0,mac=52:54:00:12:34:56,netdev=TAP0" \
img/vdisk0.img \
-device "vfio-pci,id=vfio0004_04_00_0,host=0004:04:00.0" \
-device "vfio-pci,id=vfio0006_00_00_0,host=0006:00:00.0" \
-device "vfio-pci,id=vfio0006_00_00_1,host=0006:00:00.1" \
-device "vfio-pci,id=vfio0006_00_00_2,host=0006:00:00.2" \
-device "vfio-pci,id=vfio0004_05_00_0,host=0004:05:00.0" \
-device "vfio-pci,id=vfio0006_00_01_0,host=0006:00:01.0" \
-device "vfio-pci,id=vfio0006_00_01_1,host=0006:00:01.1" \
-device "vfio-pci,id=vfio0006_00_01_2,host=0006:00:01.2" \
-device spapr-pci-host-bridge,id=phb1,index=1 \
-device "vfio-pci,id=vfio0035_03_00_0,host=0035:03:00.0" \
-device "vfio-pci,id=vfio0007_00_00_0,host=0007:00:00.0" \
-device "vfio-pci,id=vfio0007_00_00_1,host=0007:00:00.1" \
-device "vfio-pci,id=vfio0007_00_00_2,host=0007:00:00.2" \
-device "vfio-pci,id=vfio0035_04_00_0,host=0035:04:00.0" \
-device "vfio-pci,id=vfio0007_00_01_0,host=0007:00:01.0" \
-device "vfio-pci,id=vfio0007_00_01_1,host=0007:00:01.1" \
-device "vfio-pci,id=vfio0007_00_01_2,host=0007:00:01.2" -snapshot \
-machine pseries \
-L /home/aik/t/qemu-ppc64-bios/ -d guest_errors

Note that QEMU attaches PCI devices to the last added vPHB so first
8 devices - 4:04:00.0 till 6:00:01.2 - go to the default vPHB, and
35:03:00.0..7:00:01.2 to the vPHB with id=phb1.
---
 hw/ppc/Makefile.objs|   2 +-
 hw/vfio/pci.h   |   2 +
 include/hw/pci-host/spapr.h |  45 
 include/hw/ppc/spapr.h  |   5 +-
 hw/ppc/spapr.c  |  48 +++-
 hw/ppc/spapr_pci.c  |  19 ++
 hw/ppc/spapr_pci_nvlink2.c  | 450 
 hw/vfio/pci-quirks.c| 

Re: [Qemu-devel] [PATCH 11/11] target/hppa: call eval_interrupt() after ssm

2019-03-11 Thread Richard Henderson
On 3/11/19 8:28 PM, Richard Henderson wrote:
> On 3/11/19 12:16 PM, Sven Schnelle wrote:
>> HP-UX (all versions) is losing timer interrupts, which leads to
>> hangs. Pressing a key on the console fixes this, so it looks like
>> QEMU is just looping trough TBs without checking for interrupts.
>> Further investion showed that this happens when interrupts are
>> triggered, without PSW_I enabled. Calling eval_interrupt() after
>> PSW_I is set seems to fix this.
>>
>> Signed-off-by: Sven Schnelle 
>> ---
>>  target/hppa/cpu.h| 1 +
>>  target/hppa/int_helper.c | 2 +-
>>  target/hppa/op_helper.c  | 6 ++
>>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> The correct fix is to exit to the main loop.

... except what we're already doing that.  So I don't see what
can be changed to help.  This doesn't seem to make a difference.


r~



Re: [Qemu-devel] [PULL 04/60] Revert "spapr: support memory unplug for qtest"

2019-03-11 Thread David Gibson
On Mon, Mar 11, 2019 at 11:52:28AM +0100, Greg Kurz wrote:
> On Sun, 10 Mar 2019 19:26:07 +1100
> David Gibson  wrote:
> 
> > From: Greg Kurz 
> > 
> > Commit b8165118f52c broke CPU hotplug tests for old machine types:
> > 
> > $ QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 ./tests/cpu-plug-test 
> > -m=slow
> > /ppc64/cpu-plug/pseries-3.1/device-add/2x3x1=6: OK
> > /ppc64/cpu-plug/pseries-2.12-sxxm/device-add/2x3x1=6: OK
> > /ppc64/cpu-plug/pseries-3.0/device-add/2x3x1=6: OK
> > /ppc64/cpu-plug/pseries-2.10/device-add/2x3x1=6: OK
> > /ppc64/cpu-plug/pseries-2.11/device-add/2x3x1=6: OK
> > /ppc64/cpu-plug/pseries-2.12/device-add/2x3x1=6: OK
> > /ppc64/cpu-plug/pseries-2.9/device-add/2x3x1=6: OK
> > /ppc64/cpu-plug/pseries-2.7/device-add/2x3x1=6: **
> > ERROR:/home/thuth/devel/qemu/hw/ppc/spapr_events.c:313:rtas_event_log_to_source:
> >  assertion failed: (source->enabled)
> > Broken pipe
> > /home/thuth/devel/qemu/tests/libqtest.c:143: kill_qemu() detected QEMU 
> > death from signal 6 (Aborted) (core dumped)
> > Aborted (core dumped)
> > 
> > The approach of faking the availability of OV5_HP_EVT causes the
> > code to assume the hotplug event source is enabled, which is wrong
> > for older machines.
> > 
> > This reverts commit b8165118f52ce5ee88565d3cec83d30374efdc96.
> > 
> > A subsequent patch will address the problem of CAS under qtest from
> > a different angle.
> > 
> 
> Since the patches got re-ordered, this sentence is wrong. In case
> you re-send this pull request, maybe you can update the changelog
> accordingly ?

Done.

> 
> > Reported-by: Thomas Huth 
> > Signed-off-by: Greg Kurz 
> > Message-Id: <155146875097.147873.1732264036668112686.st...@bahia.lan>
> > Tested-by: Michael Roth 
> > Reviewed-by: Michael Roth 
> > Signed-off-by: David Gibson 
> > ---
> >  hw/ppc/spapr_ovec.c | 6 --
> >  1 file changed, 6 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_ovec.c b/hw/ppc/spapr_ovec.c
> > index 12510b236a..318bf33de4 100644
> > --- a/hw/ppc/spapr_ovec.c
> > +++ b/hw/ppc/spapr_ovec.c
> > @@ -16,7 +16,6 @@
> >  #include "qemu/bitmap.h"
> >  #include "exec/address-spaces.h"
> >  #include "qemu/error-report.h"
> > -#include "sysemu/qtest.h"
> >  #include "trace.h"
> >  #include 
> >  
> > @@ -132,11 +131,6 @@ bool spapr_ovec_test(sPAPROptionVector *ov, long bitnr)
> >  g_assert(ov);
> >  g_assert(bitnr < OV_MAXBITS);
> >  
> > -/* support memory unplug for qtest */
> > -if (qtest_enabled() && bitnr == OV5_HP_EVT) {
> > -return true;
> > -}
> > -
> >  return test_bit(bitnr, ov->bitmap) ? true : false;
> >  }
> >  
> 

-- 
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] [PULL 00/60] ppc-for-4.0 queue 20190310

2019-03-11 Thread David Gibson
On Mon, Mar 11, 2019 at 10:40:54AM +, Alex Bennée wrote:
> 
> Peter Maydell  writes:
> 
> > On Sun, 10 Mar 2019 at 08:27, David Gibson  
> > wrote:
> >>
> >> The following changes since commit 
> >> f5b4c31030f45293bb4517445722768434829d91:
> >>
> >>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' 
> >> into staging (2019-03-09 17:35:48 +)
> >>
> >> are available in the Git repository at:
> >>
> >>   git://github.com/dgibson/qemu.git tags/ppc-for-4.0-20190310
> >>
> >> for you to fetch changes up to 08d020471fcd41cb020fc9987ed1945eefcc8805:
> >>
> >>   spapr: Use CamelCase properly (2019-03-10 14:35:44 +1100)
> >>
> >> 
> >> ppc patch queue for 2019-03-10
> >>
> >> Here's a final pull request before the 4.0 soft freeze.  Changes
> >> include:
> >>   * A Great Renaming to use camel case properly in spapr code
> >>   * Optimization of some vector instructions
> >>   * Support for POWER9 cpus in the powernv machine
> >>   * Fixes a regression from the last pull request in handling VSX
> >> instructions with mixed operands from the FPR and VMX parts of the
> >> register array
> >>   * Optimization hack to avoid scanning all the (empty) entries on a
> >> new IOMMU window
> >>   * Add FSL I2C controller model for E500
> >>   * Support for KVM acceleration of the H_PAGE_INIT hypercall on spapr
> >>   * Update u-boot image for E500
> >>   * Enable Specre/Meltdown mitigations by default on the new machine type
> >>   * Enable large decrementer support for POWER9
> >>
> >> Plus a number of assorted bugfixes and cleanups.
> >>
> >
> > Hi. This pullreq generates a pile of new 'warning' messages
> > during 'make check':
> >
> > MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> > QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64
> > QTEST_QEMU_IMG=qemu-img tests/
> > boot-serial-test -m=quick -k --tap < /dev/null |
> > ./scripts/tap-driver.pl --test-name="boot-serial-test"
> > PASS 1 boot-serial-test /ppc64/boot-serial/ppce500
> > PASS 2 boot-serial-test /ppc64/boot-serial/40p
> > PASS 3 boot-serial-test /ppc64/boot-serial/mac99
> > qemu-system-ppc64: warning: TCG doesn't support requested feature,
> > cap-cfpc=workaround
> > qemu-system-ppc64: warning: TCG doesn't support requested feature,
> > cap-sbbc=workaround
> > qemu-system-ppc64: warning: TCG doesn't support requested feature,
> > cap-ibs=workaround
> > PASS 4 boot-serial-test /ppc64/boot-serial/pseries
> > PASS 5 boot-serial-test /ppc64/boot-serial/powernv
> > PASS 6 boot-serial-test /ppc64/boot-serial/sam460ex
> >
> > and similarly during the boot-pxe-test.
> >
> > Could you silence these, please?
> 
> FWIW this PR contains fixes that will finally get the gitlab CI green so
> I look forward to v2 getting merged ;-)

Huh.  I knew about the travis CI and the shippable CI, but not the
gitlab one.  How do I see that?  It was surprisingly non-obvious from
the gitlab qemu project page.

-- 
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] Flatview rendering scalability issue

2019-03-11 Thread Peter Xu
On Mon, Mar 11, 2019 at 03:07:43PM +0100, Paolo Bonzini wrote:
> On 11/03/19 14:48, Sergio Lopez wrote:
> >> The initialization is O(n^2) because the guest initializes one device at
> >> a time, so you rebuild the FlatView first with 0 devices, then 1, then
> >> 2, etc.  This is very hard to fix, if at all possible.
> >>
> >> However, each FlatView creation should be O(n) where n is the number of
> >> devices currently configured.  Please check with "info mtree -f" that
> >> you only have a fixed number of FlatViews.  Old versions had one per 
> >> device.
> > I'm seeing 9 FVs with 1 PCI, and 119 with 100 PCIs.
> 
> With
> 
> $ eval qemu-system-x86_64 -M q35 \
> -device\ e1000,id=n{1,2,3,4,5,6,7,8}{1,2,3}
> 
> I only see 4 flat views ("system", "io", "memory", "(none)").
> 
> Probably you are using intel-iommu?  Peter, it should be possible to
> reorganize the VT-d memory regions like this:
> 
> intel_iommu_ir (MMIO, not added to any container)
> 
> vtd_root_dmar (container)
>   intel_iommu_dmar (IOMMU), priority 0
>   alias to intel_iommu_ir, priority 1
> 
> vtd_root_nodmar
>   alias to get_system_memory(), priority 0
>   alias to intel_iommu_ir, priority 1
> 
> vtd_root_0 memory region (container)
> vtd_root_dmar # only one of these is enabled
> vtd_root_nodmar
> 
> where the vtd_root_dmar and vtd_root_nodmar memory regions are created
> in vtd_init once and for all.  Because all vtd_root_* memory regions
> have only one child, memory.c will recognize that they represent the
> same memory, and create at most two FlatViews (one for vtd_root_dmar,
> one for vtd_root_nodmar).

Yes this sounds good.  The only thing I'm still uncertain is about the
IOMMU notifiers, which should be per-device (for real).  That's
embedded in IOMMUMemoryRegion so far and it includes the real MR
object:

struct IOMMUMemoryRegion {
MemoryRegion parent_obj;
QLIST_HEAD(, IOMMUNotifier) iommu_notify;
IOMMUNotifierFlag iommu_notify_flags;
};

Maybe I should also make parent_obj a pointer to the created MRs
mentioned above, so IOMMUMemoryRegion only contains notification
information rather than real MRs (otherwise we won't have a chance to
share memory regions between devices)?

(But if so then TYPE_INTEL_IOMMU_MEMORY_REGION might not be able to
 inherit TYPE_IOMMU_MEMORY_REGION directly, and I've not thought about
 the details of that, yet)

Regards,

-- 
Peter Xu



Re: [Qemu-devel] [PATCH v2 2/4] block/qcow2-bitmap: Don't check size for IN_USE bitmap

2019-03-11 Thread Eric Blake
On 3/11/19 1:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> We are going to allow image resize when there are persistent bitmaps.
> It may lead to appearing of inconsistent bitmaps (IN_USE=1) with
> inconsistent size. But we still want to load them as inconsistent.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block/qcow2-bitmap.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)

> +{
> +/*
> + * We've loaded valid bitmap (IN_USE not set) or we are going to 
> store
> + * valid bitmap. But allocated bitmap table size is not enough to 
> store
> + * such bitmap.
> + *
> + * Note, that it's OK to have invalid bitmap with invalid size 
> during to

s/during/due/

> + * bitmap was not correctly saved after image resize.

s/bitmap/a bitmap that/

> + */
> +return -EINVAL;
> +}
> +
> +return 0;
>  }
>  
>  static inline void bitmap_directory_to_be(uint8_t *dir, size_t size)
> 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 02/11] target/hppa: fix TLB handling for page 0

2019-03-11 Thread Richard Henderson
On 3/11/19 12:15 PM, Sven Schnelle wrote:
> Assume the following sequence:
> 
> pitlbe r0(sr0,r0)
> iitlba r4,(sr0,r0)
> ldil L%300,r5
> iitlbp r5,(sr0,r0)
> 
> This will purge the whole TLB and add an entry for page 0. However
> the current TLB implementation in helper_iitlba() will store to
> the last empty TLB entry, while helper_iitlbp() will write to the
> first empty entry. That is because an empty entry will match address
> 0 in helper_iitlba()
> 
> Signed-off-by: Sven Schnelle 
> ---
>  target/hppa/mem_helper.c | 14 --
>  1 file changed, 8 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PULL 00/60] ppc-for-4.0 queue 20190310

2019-03-11 Thread David Gibson
On Sun, Mar 10, 2019 at 04:06:38PM +, Peter Maydell wrote:
> On Sun, 10 Mar 2019 at 08:27, David Gibson  
> wrote:
> >
> > The following changes since commit f5b4c31030f45293bb4517445722768434829d91:
> >
> >   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' 
> > into staging (2019-03-09 17:35:48 +)
> >
> > are available in the Git repository at:
> >
> >   git://github.com/dgibson/qemu.git tags/ppc-for-4.0-20190310
> >
> > for you to fetch changes up to 08d020471fcd41cb020fc9987ed1945eefcc8805:
> >
> >   spapr: Use CamelCase properly (2019-03-10 14:35:44 +1100)
> >
> > 
> > ppc patch queue for 2019-03-10
> >
> > Here's a final pull request before the 4.0 soft freeze.  Changes
> > include:
> >   * A Great Renaming to use camel case properly in spapr code
> >   * Optimization of some vector instructions
> >   * Support for POWER9 cpus in the powernv machine
> >   * Fixes a regression from the last pull request in handling VSX
> > instructions with mixed operands from the FPR and VMX parts of the
> > register array
> >   * Optimization hack to avoid scanning all the (empty) entries on a
> > new IOMMU window
> >   * Add FSL I2C controller model for E500
> >   * Support for KVM acceleration of the H_PAGE_INIT hypercall on spapr
> >   * Update u-boot image for E500
> >   * Enable Specre/Meltdown mitigations by default on the new machine type
> >   * Enable large decrementer support for POWER9
> >
> > Plus a number of assorted bugfixes and cleanups.
> >
> 
> Hi. This pullreq generates a pile of new 'warning' messages
> during 'make check':
> 
> MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
> QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64
> QTEST_QEMU_IMG=qemu-img tests/
> boot-serial-test -m=quick -k --tap < /dev/null |
> ./scripts/tap-driver.pl --test-name="boot-serial-test"
> PASS 1 boot-serial-test /ppc64/boot-serial/ppce500
> PASS 2 boot-serial-test /ppc64/boot-serial/40p
> PASS 3 boot-serial-test /ppc64/boot-serial/mac99
> qemu-system-ppc64: warning: TCG doesn't support requested feature,
> cap-cfpc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature,
> cap-sbbc=workaround
> qemu-system-ppc64: warning: TCG doesn't support requested feature,
> cap-ibs=workaround
> PASS 4 boot-serial-test /ppc64/boot-serial/pseries
> PASS 5 boot-serial-test /ppc64/boot-serial/powernv
> PASS 6 boot-serial-test /ppc64/boot-serial/sam460ex
> 
> and similarly during the boot-pxe-test.
> 
> Could you silence these, please?

Ok, done.  As a rule these warnings are there intentionally for TCG -
we want to enable Spectre/Meltdown mitigations by default, but no-one
really knows if and how to implement them for TCG.

But I can and have suppressed the warnings for the qtest case.

-- 
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 03/11] target/hppa: report ITLB_EXCP_MISS for ITLB misses

2019-03-11 Thread Richard Henderson
On 3/11/19 12:15 PM, Sven Schnelle wrote:
> Signed-off-by: Sven Schnelle 
> ---
>  target/hppa/mem_helper.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

At one point this didn't boot linux, but perhaps there was a second bug.

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH 01/11] target/hppa: fix overwriting source reg in addb

2019-03-11 Thread Richard Henderson
On 3/11/19 12:15 PM, Sven Schnelle wrote:
> When one of the source registers is the same as the destination register,
> the source register gets overwritten with the destionation value before
> do_add_sv() is called, which leads to unexpection condition matches.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  target/hppa/translate.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~



Re: [Qemu-devel] [PATCH 05/11] target/hppa: remove PSW I/R/Q bit check

2019-03-11 Thread Richard Henderson
On 3/11/19 12:15 PM, Sven Schnelle wrote:
> HP ODE use rfi to set the Q bit, and i don't see anything in the
> documentation that this is forbidden. So remove it.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  target/hppa/op_helper.c | 5 -
>  1 file changed, 5 deletions(-)

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH 04/11] target/hppa: add TLB trace events

2019-03-11 Thread Richard Henderson
On 3/11/19 12:15 PM, Sven Schnelle wrote:
> To ease TLB debugging add a few trace events, which are disabled
> by default so that there's no performance impact.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  Makefile.objs|  1 +
>  target/hppa/mem_helper.c | 20 ++--
>  target/hppa/op_helper.c  |  2 ++
>  target/hppa/trace-events | 18 ++
>  4 files changed, 39 insertions(+), 2 deletions(-)
>  create mode 100644 target/hppa/trace-events

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH 09/11] target/hppa: add TLB protection id check

2019-03-11 Thread Richard Henderson
On 3/11/19 12:16 PM, Sven Schnelle wrote:
> +/* access_id == 0 means public page and no check is performed */
> +if ((env->psw & PSW_P) && ent->access_id) {
> +wd = 1;
> +
> +if (ent->access_id == (env->cr[CR_PID1] >> 1)) {
> +wd &= env->cr[CR_PID1];
> +}
> +
> +if (ent->access_id == (env->cr[CR_PID2] >> 1)) {
> +wd &= env->cr[CR_PID2];
> +}
> +
> +if (ent->access_id == (env->cr[CR_PID3] >> 1)) {
> +wd &= env->cr[CR_PID3];
> +}
> +
> +if (ent->access_id == (env->cr[CR_PID4] >> 1)) {
> +wd &= env->cr[CR_PID4];
> +}
> +
> +if (wd && (type & w_prot)) {
> +ret = EXCP_DMPI;
> +goto egress;
> +}
> +}

This is insufficient.

(1) The softmmu tlb much be flushed when PSW_P,
or any of the PID registers change.
(2) If type != PAGE_WRITE, you need to remove PAGE_WRITE
from prot so that the next write doesn't see wrong permissions.

I'll be testing something like the following.


r~


diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 861bbb1f16..c062c7969c 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -143,6 +143,10 @@
 #endif
 
 #define CR_RC0
+#define CR_PID1  8
+#define CR_PID2  9
+#define CR_PID3  12
+#define CR_PID4  13
 #define CR_SCRCCR10
 #define CR_SAR   11
 #define CR_IVA   14
@@ -341,6 +345,12 @@ target_ureg cpu_hppa_get_psw(CPUHPPAState *env);
 void cpu_hppa_put_psw(CPUHPPAState *env, target_ureg);
 void cpu_hppa_loaded_fr0(CPUHPPAState *env);
 
+#ifdef CONFIG_USER_ONLY
+static inline void cpu_hppa_change_prot_id(CPUHPPAState *env) { }
+#else
+void cpu_hppa_change_prot_id(CPUHPPAState *env);
+#endif
+
 #define cpu_signal_handler cpu_hppa_signal_handler
 
 int cpu_hppa_signal_handler(int host_signum, void *pinfo, void *puc);
diff --git a/target/hppa/helper.h b/target/hppa/helper.h
index bfe0dd1db1..38d834ef6b 100644
--- a/target/hppa/helper.h
+++ b/target/hppa/helper.h
@@ -92,4 +92,5 @@ DEF_HELPER_FLAGS_3(itlbp, TCG_CALL_NO_RWG, void, env, tl, tr)
 DEF_HELPER_FLAGS_2(ptlb, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_FLAGS_1(ptlbe, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(lpa, TCG_CALL_NO_WG, tr, env, tl)
+DEF_HELPER_FLAGS_1(change_prot_id, TCG_CALL_NO_RWG, void, env)
 #endif
diff --git a/target/hppa/gdbstub.c b/target/hppa/gdbstub.c
index 3157a690f2..983bf92aaf 100644
--- a/target/hppa/gdbstub.c
+++ b/target/hppa/gdbstub.c
@@ -93,19 +93,19 @@ int hppa_cpu_gdb_read_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 val = env->cr[CR_RC];
 break;
 case 52:
-val = env->cr[8];
+val = env->cr[CR_PID1];
 break;
 case 53:
-val = env->cr[9];
+val = env->cr[CR_PID2];
 break;
 case 54:
 val = env->cr[CR_SCRCCR];
 break;
 case 55:
-val = env->cr[12];
+val = env->cr[CR_PID3];
 break;
 case 56:
-val = env->cr[13];
+val = env->cr[CR_PID4];
 break;
 case 57:
 val = env->cr[24];
@@ -224,19 +224,23 @@ int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 env->cr[CR_RC] = val;
 break;
 case 52:
-env->cr[8] = val;
+env->cr[CR_PID1] = val;
+cpu_hppa_change_prot_id(env);
 break;
 case 53:
-env->cr[9] = val;
+env->cr[CR_PID2] = val;
+cpu_hppa_change_prot_id(env);
 break;
 case 54:
 env->cr[CR_SCRCCR] = val;
 break;
 case 55:
-env->cr[12] = val;
+env->cr[CR_PID3] = val;
+cpu_hppa_change_prot_id(env);
 break;
 case 56:
-env->cr[13] = val;
+env->cr[CR_PID4] = val;
+cpu_hppa_change_prot_id(env);
 break;
 case 57:
 env->cr[24] = val;
diff --git a/target/hppa/helper.c b/target/hppa/helper.c
index 6539061e52..ac750b62ef 100644
--- a/target/hppa/helper.c
+++ b/target/hppa/helper.c
@@ -21,6 +21,7 @@
 
 #include "cpu.h"
 #include "fpu/softfloat.h"
+#include "exec/exec-all.h"
 #include "exec/helper-proto.h"
 
 target_ureg cpu_hppa_get_psw(CPUHPPAState *env)
@@ -49,6 +50,7 @@ target_ureg cpu_hppa_get_psw(CPUHPPAState *env)
 
 void cpu_hppa_put_psw(CPUHPPAState *env, target_ureg psw)
 {
+target_ureg old_psw = env->psw;
 target_ureg cb = 0;
 
 env->psw = psw & ~(PSW_N | PSW_V | PSW_CB);
@@ -64,6 +66,14 @@ void cpu_hppa_put_psw(CPUHPPAState *env, target_ureg psw)
 cb |= ((psw >>  9) & 1) <<  8;
 cb |= ((psw >>  8) & 1) <<  4;
 env->psw_cb = cb;
+
+/* If PSW_P changes, it affects how we translate addresses.  */
+if ((psw ^ old_psw) & PSW_P) {
+#ifndef CONFIG_USER_ONLY
+CPUState *src = CPU(hppa_env_get_cpu(env));
+tlb_flush_by_mmuidx(src, 0xf);
+#endif
+}
 }
 
 void hppa_cpu_dump_state(CPUState 

Re: [Qemu-devel] [PATCH 10/11] target/hppa: exit TB if either Data or Instruction TLB changes

2019-03-11 Thread Richard Henderson
On 3/11/19 12:16 PM, Sven Schnelle wrote:
> The current code assumes that we don't need to exit the TB
> if a Data Cache Flush or Insert has happend. However, as we
> have a shared Data/Instruction TLB, a Data cache flush also
> flushes Instruction TLB entries, and a Data cache TLB insert
> might also evict a Instruction TLB entry.
> 
> So exit the TB in all cases if Instruction translation is enabled.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  target/hppa/translate.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)

I suppose since we advertise a unified i/d tlb, the os feels that
either i/d flush should be sufficient.

Reviewed-by: Richard Henderson 


r~




Re: [Qemu-devel] [PATCH 11/11] target/hppa: call eval_interrupt() after ssm

2019-03-11 Thread Richard Henderson
On 3/11/19 12:16 PM, Sven Schnelle wrote:
> HP-UX (all versions) is losing timer interrupts, which leads to
> hangs. Pressing a key on the console fixes this, so it looks like
> QEMU is just looping trough TBs without checking for interrupts.
> Further investion showed that this happens when interrupts are
> triggered, without PSW_I enabled. Calling eval_interrupt() after
> PSW_I is set seems to fix this.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  target/hppa/cpu.h| 1 +
>  target/hppa/int_helper.c | 2 +-
>  target/hppa/op_helper.c  | 6 ++
>  3 files changed, 8 insertions(+), 1 deletion(-)

The correct fix is to exit to the main loop.


r~



Re: [Qemu-devel] [PATCH 0/5] QEMU VFIO live migration

2019-03-11 Thread Zhao Yan
hi Alex
thanks for your reply.

So, if we choose migration data to be userspace opaque, do you think below
sequence is the right behavior for vendor driver to follow:

1. initially LOGGING state is not set. If userspace calls GET_BUFFER to
vendor driver,  vendor driver should reject and return 0.

2. then LOGGING state is set, if userspace calls GET_BUFFER to vendor
driver,
   a. vendor driver shoud first query a whole snapshot of device memory
   (let's use this term to represent device's standalone memory for now),
   b. vendor driver returns a chunk of data just queried to userspace,
   while recording current pos in data.
   c. vendor driver finds all data just queried is finished transmitting to
   userspace, and queries only dirty data in device memory now.
   d. vendor driver returns a chunk of data just quered (this time is dirty
   data )to userspace while recording current pos in data
   e. if all data is transmited to usespace and still GET_BUFFERs come from
   userspace, vendor driver starts another round of dirty data query.

3. if LOGGING state is unset then, and userpace calls GET_BUFFER to vendor
driver,
   a. if vendor driver finds there's previously untransmitted data, returns
   them until all transmitted.
   b. vendor driver then queries dirty data again and transmits them.
   c. at last, vendor driver queris device config data (which has to be
   queried at last and sent once) and transmits them.


for the 1 bullet, if LOGGING state is firstly set and migration aborts
then,  vendor driver has to be able to detect that condition. so seemingly,
vendor driver has to know more qemu's migration state, like migration
called and failed. Do you think that's acceptable?


Thanks
Yan





Re: [Qemu-devel] [PATCH v2 1/4] docs/interop/qcow2: Improve bitmap flag in_use specification

2019-03-11 Thread Eric Blake
On 3/11/19 1:51 PM, Vladimir Sementsov-Ogievskiy wrote:
> We already use (we didn't notice it) IN_USE flag for marking bitmap
> metadata outdated, such as AUTO flag, which mirrors enabled/disabled
> bitmaps. No we are going to support bitmap resize, so it's good to

s/No/Now/

> write IN_USE meaning with more details.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  docs/interop/qcow2.txt | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
> index fb5cb47245..575a5f25e2 100644
> --- a/docs/interop/qcow2.txt
> +++ b/docs/interop/qcow2.txt
> @@ -589,7 +589,19 @@ Structure of a bitmap directory entry:
>  Bit
>0: in_use
>   The bitmap was not saved correctly and may be
> - inconsistent.
> + inconsistent. This inconsitency may touch both 
> bitmap

inconsistency

> + data and metadata, and this mean that bitmap state
> + (its data and metadata) was changed but not stored
> + back to the image. This flag doesn't relate to 
> format
> + corruption, all fields are still correct from qcow2
> + point of view, they just may be outdated.
> +
> + Note: Currently, Qemu may change (additionally to
> + bitmap data) @auto flag and size of the bitmap 
> during
> + image resize. This mean, that not only bitmap data
> + may be outdated if @in_use flag set, but also value 
> of
> + @auto flag and bitmap size (which is indirectly
> + referenced by @bitmap_table_size).

Feels wordy. Maybe drop the second paragraph starting with Note, and
merely use this for the first paragraph:

The bitmap was not saved correctly and may be inconsistent. Although the
bitmap metadata is still well-formed from a qcow2 perspective, the
metadata (such as the auto flag or bitmap size) or data contents may be
outdated.

>  
>1: auto
>   The bitmap must reflect all changes of the virtual
> @@ -717,8 +729,8 @@ corresponding range of the virtual disk (see above) was 
> written to while the
>  bitmap was 'enabled'. An unset bit means that this range was not written to.
>  
>  The software doesn't have to sync the bitmap in the image file with its
> -representation in RAM after each write. Flag 'in_use' should be set while the
> -bitmap is not synced.
> +representation in RAM after each write or metadata change. Flag 'in_use'
> +should be set while the bitmap is not synced.
>  
>  In the image file the 'enabled' state is reflected by the 'auto' flag. If 
> this
>  flag is set, the software must consider the bitmap as 'enabled' and start
> 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2 03/10] block: Avoid useless local_err

2019-03-11 Thread Eric Blake
On 3/11/19 11:50 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  block.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 

Reviewed-by: Eric Blake 

> diff --git a/block.c b/block.c
> index ccf008c177..e18bd5eefd 100644
> --- a/block.c
> +++ b/block.c
> @@ -3155,14 +3155,12 @@ int bdrv_reopen_multiple(AioContext *ctx, 
> BlockReopenQueue *bs_queue, Error **er
>  {
>  int ret = -1;
>  BlockReopenQueueEntry *bs_entry, *next;
> -Error *local_err = NULL;
>  
>  assert(bs_queue != NULL);
>  
>  QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
>  assert(bs_entry->state.bs->quiesce_counter > 0);
> -if (bdrv_reopen_prepare(_entry->state, bs_queue, _err)) {
> -error_propagate(errp, local_err);
> +if (bdrv_reopen_prepare(_entry->state, bs_queue, errp)) {
>  goto cleanup;
>  }
>  bs_entry->prepared = true;
> 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2 02/10] qemu-iotests: commit to backing file with auto-read-only

2019-03-11 Thread Eric Blake
On 3/11/19 11:50 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf 
> ---
>  tests/qemu-iotests/232 | 31 +++
>  tests/qemu-iotests/232.out | 20 
>  2 files changed, 51 insertions(+)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 1/8] tests/virtio-blk-test: Disable auto-read-only

2019-03-11 Thread Eric Blake
On 3/8/19 9:37 AM, Kevin Wolf wrote:
> tests/virtio-blk-test uses a temporary image file that it deletes while
> QEMU is still running, so it can't be reopened when writers are
> attached or detached. Disable auto-read-only to keep it always writable.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  tests/virtio-blk-test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake 

> 
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 8d2fc9c710..0e464aeea4 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -72,7 +72,7 @@ static QOSState *pci_test_start(void)
>  QOSState *qs;
>  const char *arch = qtest_get_arch();
>  char *tmp_path;
> -const char *cmd = "-drive if=none,id=drive0,file=%s,format=raw "
> +const char *cmd = "-drive 
> if=none,id=drive0,file=%s,format=raw,auto-read-only=off "
>"-drive if=none,id=drive1,file=null-co://,format=raw "
>"-device virtio-blk-pci,id=drv0,drive=drive0,"
>"addr=%x.%x";
> 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2 01/10] tests/virtio-blk-test: Disable auto-read-only

2019-03-11 Thread Eric Blake
On 3/11/19 11:50 AM, Kevin Wolf wrote:
> tests/virtio-blk-test uses a temporary image file that it deletes while
> QEMU is still running, so it can't be reopened when writers are
> attached or detached. Disable auto-read-only to keep it always writable.
> 
> Signed-off-by: Kevin Wolf 
> ---
>  tests/virtio-blk-test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Looks like I started reviewing v1 even though I see v2 has been posted.
R-b carries forward:

Reviewed-by: Eric Blake 

> 
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index b02be0274e..b65365934b 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -751,7 +751,7 @@ static void *virtio_blk_test_setup(GString *cmd_line, 
> void *arg)
>  char *tmp_path = drive_create();
>  
>  g_string_append_printf(cmd_line,
> -   " -drive if=none,id=drive0,file=%s,format=raw "
> +   " -drive 
> if=none,id=drive0,file=%s,format=raw,auto-read-only=off "
> "-drive 
> if=none,id=drive1,file=null-co://,format=raw ",
> tmp_path);
>  
> 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 0/5] QEMU VFIO live migration

2019-03-11 Thread Tian, Kevin
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Tuesday, March 12, 2019 4:19 AM
> On Mon, 11 Mar 2019 02:33:11 +
> "Tian, Kevin"  wrote:
> 
[...]
> 
> I think I've fully conceded any notion of security by obscurity towards
> opaque data already, but segregating types of device data still seems
> to unnecessarily impose a usage model on the vendor driver that I think
> we should try to avoid.  The migration interface should define the
> protocol through which userspace can save and restore the device state,
> not impose how the vendor driver exposes or manages that state.  Also, I
> got the impression (perhaps incorrectly) that you were trying to mmap
> live data to userspace, which would allow not only saving the state,
> but also unchecked state modification by userspace. I think we want
> more of a producer/consumer model of the state where consuming state
> also involves at least some degree of sanity or consistency checking.
> Let's not forget too that we're obviously dealing with non-open source
> driver in the mdev universe as well.

OK. I think for this part we are in agreement - as long as the goal of
this interface is clearly defined as such way. :-)

[...]
> > But I still didn't get your exact concern about security part. For
> > version yes we still haven't worked out a sane way to represent
> > vendor-specific compatibility requirement. But allowing user
> > space to modify data through this interface has really no difference
> > from allowing guest to modify data through trapped MMIO interface.
> > mdev driver should guarantee that operations through both interfaces
> > can modify only the state associated with the said mdev instance,
> > w/o breaking the isolation boundary. Then the former becomes just
> > a batch of operations to be verified in the same way as if they are
> > done individually through the latter interface.
> 
> It seems like you're assuming a working model for the vendor driver and
> the data entering and exiting through this interface.  The vendor
> drivers can expose data any way that they want.  All we need to do is
> imagine that the migration data stream includes an array index count
> somewhere which the user could modify to trigger the in-kernel vendor
> driver to allocate an absurd array size and DoS the target.  This is
> probably the most simplistic attack, possibly knowing the state machine
> of the vendor driver a malicious user could trick it into providing
> host kernel data.  We're not necessarily only conveying state that the
> user already has access to via this interface, the vendor driver may
> include non-visible internal state as well.  Even the state that is
> user accessible is being pushed into the vendor driver via an alternate
> path from the user mediation we have on the existing paths.

Then I don't know how this concern can be effectively addressed 
since you assume vendor drivers are not trusted here. and why do
you trust vendor drivers on mediating existing path but not this
alternative one? non-visible internal states just mean more stuff
to be carefully scrutinized, which is not essentially causing a 
conceptual difference of trust level.

Or can this concern be partially mitigated if we create some 
test cases which poke random data through the new interface,
and mark vendor drivers which pass such tests as trusted? Then
there is also an open who should be in charge of such certification 
process...

Thanks
Kevin



Re: [Qemu-devel] [Qemu-block] [PATCH] tcmu: Introduce qemu-tcmu utility

2019-03-11 Thread Fam Zheng



> On Mar 11, 2019, at 19:06, Kevin Wolf  wrote:
> 
> Am 09.03.2019 um 02:46 hat Yaowei Bai geschrieben:
>> Thanks for explaining the background. It comes to my mind that actually we
>> talked about these two cases with Fam a bit long time ago and decided to
>> support both these two cases. The reason why we implement case2 first is
>> currently we care more about exporting new opened images and it's a bit
>> more convenient, exporting from a VM or QMP can be added in the later
>> release. Do you think it's reasonable/acceptable that we support both
>> cases and use case2 for normal new opened images and case1 for the
>> circumstances you mention above?
> 
> I would like to avoid a second code path because it comes with a
> maintenance cost.
> 
> Experience also tells that adding a new way to parse option strings will
> come back at us later because it we must always maintain compatibility
> with yet another format.

If the rule is that cfgstr strictly follows -blockdev syntax and the parsing 
code is mostly shared, it shouldn’t be a problem, right? I suppose this will 
limit the expressiveness of cfgstr but might be a reasonable tradeoff between 
implementation complexity, user friendliness and interface consistency.

Another possibility is to use json: format in cfgstr for anything more complex 
than a single -blockdev.

> 
> So I would prefer not doing this and just passing command line options
> to qemu-tcmu, which can reuse the already existing code paths.

I think the effect of not supporting adding new blockdev from cfgstr is that we 
have to resort to QMP to allow hot-adding targets. It’s not necessarily bad, 
though I’m not sure hwo that aligns with Yaowei’s development plan.

Fam


> 
> Kevin
> 





[Qemu-devel] [PULL 0/1] qmp-shell: fix nested json regression

2019-03-11 Thread Eduardo Habkost
The following changes since commit 336cfef495f0cd5b1606251c52628d0372e9a809:

  Makefile: Don't install non-sphinx files in sphinx docs install (2019-03-11 
11:10:44 +)

are available in the Git repository at:

  git://github.com/ehabkost/qemu.git tags/python-next-pull-request

for you to fetch changes up to b35203b227261961ff1da9b77b546c1919968bee:

  qmp-shell: fix nested json regression (2019-03-11 10:22:31 -0300)


qmp-shell: fix nested json regression

One small bug fix.



Marc-André Lureau (1):
  qmp-shell: fix nested json regression

 scripts/qmp/qmp-shell | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140




[Qemu-devel] [PULL 1/1] qmp-shell: fix nested json regression

2019-03-11 Thread Eduardo Habkost
From: Marc-André Lureau 

Commit fcfab7541 ("qmp-shell: learn to send commands with quoted
arguments") introduces the usage of Python 'shlex' to handle quoted
arguments, but it accidentally broke generation of nested JSON
structs.

shlex drops quotes, which breaks parsing of the nested struct.

cmd='blockdev-create job-id="job0 foo" 
options={"driver":"qcow2","size":16384,"file":{"driver":"file","filename":"foo.qcow2"}}'

shlex.split(cmd)
['blockdev-create',
 'job-id=job0 foo',
 'options={driver:qcow2,size:16384,file:{driver:file,filename:foo.qcow2}}']

Replace with a regexp to split while respecting quoted strings and preserving 
quotes:

re.findall(r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+''', cmd)
['blockdev-create',
 'job-id="job0 foo"',
 
'options={"driver":"qcow2","size":16384,"file":{"driver":"file","filename":"foo.qcow2"}}']

Fixes: fcfab7541 ("qmp-shell: learn to send commands with quoted arguments")
Reported-by: Kashyap Chamarthy 
Signed-off-by: Marc-André Lureau 
Message-Id: <20190205134926.8312-1-marcandre.lur...@redhat.com>
Tested-by: Kashyap Chamarthy 
Signed-off-by: Eduardo Habkost 
---
 scripts/qmp/qmp-shell | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/qmp/qmp-shell b/scripts/qmp/qmp-shell
index 9fec46e2ed..7776c7b141 100755
--- a/scripts/qmp/qmp-shell
+++ b/scripts/qmp/qmp-shell
@@ -73,7 +73,7 @@ import sys
 import os
 import errno
 import atexit
-import shlex
+import re
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu import qmp
@@ -222,7 +222,7 @@ class QMPShell(qmp.QEMUMonitorProtocol):
 
 < command-name > [ arg-name1=arg1 ] ... [ arg-nameN=argN ]
 """
-cmdargs = shlex.split(cmdline)
+cmdargs = 
re.findall(r'''(?:[^\s"']|"(?:\\.|[^"])*"|'(?:\\.|[^'])*')+''', cmdline)
 
 # Transactional CLI entry/exit:
 if cmdargs[0] == 'transaction(':
-- 
2.18.0.rc1.1.g3f1ff2140




Re: [Qemu-devel] Question about VM inner route entry's lost when vhost-user reconnect

2019-03-11 Thread Lilijun (Jerry, Cloud Networking)



> -Original Message-
> From: Stefan Hajnoczi [mailto:stefa...@gmail.com]
> Sent: Monday, March 11, 2019 5:47 PM
> To: Lilijun (Jerry, Cloud Networking) 
> Cc: qemu-devel@nongnu.org; wangxin (U)
> ; wangyunjian
> 
> Subject: Re: [Qemu-devel] Question about VM inner route entry's lost when
> vhost-user reconnect
> 
> On Fri, Mar 08, 2019 at 02:31:12AM +, Lilijun (Jerry, Cloud Networking)
> wrote:
> > This problem is related with backend vhost-user socket abnormal cases, we
> shouldn't ask customers to configure it manually for backend's issues or
> depends on guest OS's network configuration.
> 
> In Step 1 you said:
> 
> > > >  1) In the VM, I add one route entry manually on the vNIC eth0
> > > > using the
> > > linux tool route.
> 
> You configured the route manually inside the guest.  Seems like a guest
> problem to me.
> 
> If this was a physical machine that lost connectivity due to a link event, 
> what
> would happen?

Yes, the configuration can be recovered manually by customers.

But in the virtualization machines, this configuration's lost is a result of 
backend process's software unexpected bugs or version update. So I think we 
need hide this change to customers.
> 
> Stefan



Re: [Qemu-devel] [PATCH] ui/cocoa: Adding cursor support

2019-03-11 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/be04b88b-9aac-4348-a165-e115cbe54...@me.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: be04b88b-9aac-4348-a165-e115cbe54...@me.com
Subject: [Qemu-devel] [PATCH] ui/cocoa: Adding cursor support

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]   patchew/be04b88b-9aac-4348-a165-e115cbe54...@me.com 
-> patchew/be04b88b-9aac-4348-a165-e115cbe54...@me.com
Switched to a new branch 'test'
0fa666fc9e ui/cocoa: Adding cursor support

=== OUTPUT BEGIN ===
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Chen Zhang via Qemu-devel 

total: 1 errors, 0 warnings, 121 lines checked

Commit 0fa666fc9e29 (ui/cocoa: Adding cursor support) has style problems, 
please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/be04b88b-9aac-4348-a165-e115cbe54...@me.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PATCH] ui/cocoa: Adding cursor support

2019-03-11 Thread Chen Zhang via Qemu-devel
This patch added cocoa_mouse_set() and cocoa_cursor_define(),
supporting virtio-gpu simple rendering on macOS host.

Image content, rect and visibility of the cursor buffer were added as
ivars in QemuCocoaView class. Corresponding accessors were added.

Note that the rect of the cursor was in coordinates of the QEMU
screen, not the NSScreen or the NSView, for convenience of blending.

Signed-off-by: Chen Zhang 
---
 ui/cocoa.m | 79 ++
 1 file changed, 79 insertions(+)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index 420b2411c1..8beed6e514 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -334,6 +334,9 @@ static void handleAnyDeviceErrors(Error * err)
 BOOL isFullscreen;
 BOOL isAbsoluteEnabled;
 BOOL isMouseDeassociated;
+CGRect cursorRect;
+CGImageRef cursorImage;
+BOOL cursorVisible;
 }
 - (void) switchSurface:(pixman_image_t *)image;
 - (void) grabMouse;
@@ -362,6 +365,12 @@ static void handleAnyDeviceErrors(Error * err)
 - (float) cdy;
 - (QEMUScreen) gscreen;
 - (void) raiseAllKeys;
+- (CGRect) cursorRect;
+- (void) setCursorRect:(CGRect)rect;
+- (CGImageRef) cursorImage;
+- (void) setCursorImage:(CGImageRef)image;
+- (BOOL) isCursorVisible;
+- (void) setCursorVisible:(BOOL)visible;
 @end
 
 QemuCocoaView *cocoaView;
@@ -392,6 +401,10 @@ QemuCocoaView *cocoaView;
 pixman_image_unref(pixman_image);
 }
 
+if (cursorImage) {
+CGImageRelease(cursorImage);
+}
+
 [super dealloc];
 }
 
@@ -479,6 +492,10 @@ QemuCocoaView *cocoaView;
 );
 CGContextDrawImage (viewContextRef, cgrect(rectList[i]), 
clipImageRef);
 CGImageRelease (clipImageRef);
+
+}
+if (cursorVisible && cursorImage && NSIntersectsRect(rect, 
cursorRect)) {
+CGContextDrawImage (viewContextRef, cursorRect, cursorImage);
 }
 CGImageRelease (imageRef);
 }
@@ -987,6 +1004,19 @@ QemuCocoaView *cocoaView;
 - (float) cdy {return cdy;}
 - (QEMUScreen) gscreen {return screen;}
 
+- (CGRect) cursorRect {return cursorRect;}
+- (void) setCursorRect:(CGRect)rect {cursorRect = rect;}
+- (CGImageRef) cursorImage {return cursorImage;}
+- (void) setCursorImage:(CGImageRef)image
+{
+if (cursorImage && cursorImage != image) {
+CGImageRelease(cursorImage);
+}
+cursorImage = image;
+}
+- (BOOL) isCursorVisible {return cursorVisible;}
+- (void) setCursorVisible:(BOOL)visible {cursorVisible = visible;}
+
 /*
  * Makes the target think all down keys are being released.
  * This prevents a stuck key problem, since we will not see
@@ -1830,6 +1860,53 @@ static void cocoa_refresh(DisplayChangeListener *dcl)
 [pool release];
 }
 
+static void cocoa_cursor_define(DisplayChangeListener *dcl, QEMUCursor *c)
+{
+NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+int bitsPerComponent = [cocoaView gscreen].bitsPerComponent;
+int bitsPerPixel = [cocoaView gscreen].bitsPerPixel;
+CGDataProviderRef provider = CGDataProviderCreateWithData(NULL, c->data, 
c->width * 4 * c->height, NULL);
+
+CGImageRef img = CGImageCreate(c->width, c->height,
+   bitsPerComponent, bitsPerPixel, c->width * 
bitsPerComponent / 2,
+#ifdef __LITTLE_ENDIAN__
+   
CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace for OS X >= 
10.4
+   kCGBitmapByteOrder32Little | 
kCGImageAlphaFirst,
+#else
+   CGColorSpaceCreateDeviceRGB(), //colorspace 
for OS X < 10.4 (actually ppc)
+   kCGImageAlphaFirst, //bitmapInfo
+#endif
+   provider, NULL, 0, 
kCGRenderingIntentDefault);
+
+CGDataProviderRelease(provider);
+CGFloat width = c->width;
+CGFloat height = c->height;
+dispatch_async(dispatch_get_main_queue(), ^{
+[cocoaView setCursorImage:img];
+CGRect rect = [cocoaView cursorRect];
+rect.size = CGSizeMake(width, height);
+[cocoaView setCursorRect:rect];
+});
+[pool release];
+}
+static void cocoa_mouse_set(DisplayChangeListener *dcl,
+int x, int y, int visible)
+{
+NSAutoreleasePool * pool = [[NSAutoreleasePool alloc] init];
+dispatch_async(dispatch_get_main_queue(), ^{
+QEMUScreen screen = [cocoaView gscreen];
+CGRect rect = [cocoaView cursorRect];
+// Mark old cursor rect as dirty
+[cocoaView setNeedsDisplayInRect:rect];
+rect.origin = CGPointMake(x, screen.height - (y + rect.size.height));
+[cocoaView setCursorRect:rect];
+[cocoaView setCursorVisible:visible ? YES : NO];
+// Mark new cursor rect as dirty
+[cocoaView setNeedsDisplayInRect:rect];
+});
+[pool release];
+}
+
 static void cocoa_cleanup(void)
 {
 COCOA_DEBUG("qemu_cocoa: cocoa_cleanup\n");
@@ -1841,6 

Re: [Qemu-devel] [PATCH 07/11] target/hppa: fix b,gate instruction

2019-03-11 Thread Richard Henderson
On 3/11/19 12:15 PM, Sven Schnelle wrote:
> b,gate does GR[t] ← cat(GR[t]{0..29},IAOQ_Front{30..31});
> instead of saving the link address to register t.
> 

Quite right.  Silly mistake.

>  #ifndef CONFIG_USER_ONLY
> +TCGv_reg tmp;
>  if (ctx->tb_flags & PSW_C) {
>  CPUHPPAState *env = ctx->cs->env_ptr;
>  int type = hppa_artype_for_page(env, ctx->base.pc_next);
> @@ -3480,12 +3481,13 @@ static bool trans_b_gate(DisasContext *ctx, 
> arg_b_gate *a)
>  if (type >= 4 && type - 4 < ctx->privilege) {
>  dest = deposit32(dest, 0, 2, type - 4);
>  }
> +tmp = dest_gpr(ctx, a->l);
> +tcg_gen_deposit_reg(tmp, tmp, cpu_iaoq_f, 0, 2);
>  } else {
>  dest &= -4;  /* priv = 0 */
>  }
>  #endif
> -
> -return do_dbranch(ctx, dest, a->l, a->n);
> +return do_dbranch(ctx, dest, 0, a->n);

This change needs to be outside the CONFIG_USER_ONLY.  It needs to handle
nullification (which was previously all handled in do_dbranch).  I'm thinking
of something like the following.


r~


diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index f3e78b8e22..6ac196804b 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3446,6 +3446,8 @@ static bool trans_b_gate
 {
 target_ureg dest = iaoq_dest(ctx, a->disp);

+nullify_over(ctx);
+
 /* Make sure the caller hasn't done something weird with the queue.
  * ??? This is not quite the same as the PSW[B] bit, which would be
  * expensive to track.  Real hardware will trap for
@@ -3483,7 +3485,16 @@ static bool trans_b_gate
 }
 #endif

-return do_dbranch(ctx, dest, a->l, a->n);
+if (a->l) {
+TCGv_reg tmp = dest_gpr(ctx, a->l);
+if (ctx->privilege < 3) {
+tcg_gen_andi_reg(tmp, tmp, -4);
+}
+tcg_gen_ori_reg(tmp, tmp, ctx->privilege);
+save_gpr(ctx, a->l, tmp);
+}
+
+return do_dbranch(ctx, dest, 0, a->n);
 }

 static bool trans_blr(DisasContext *ctx, arg_blr *a)




Re: [Qemu-devel] [PATCH 08/11] target/hppa: allow multiple itlbp without itlba

2019-03-11 Thread Richard Henderson
On 3/11/19 12:15 PM, Sven Schnelle wrote:
> The ODE software calls itlbp on existing TLB entries without
> calling itlba first, so this seems to be valid.
> 
> Signed-off-by: Sven Schnelle 
> ---
>  target/hppa/mem_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
> index 26da953185..fc1b6a4fcd 100644
> --- a/target/hppa/mem_helper.c
> +++ b/target/hppa/mem_helper.c
> @@ -277,7 +277,7 @@ void HELPER(itlbp)(CPUHPPAState *env, target_ulong addr, 
> target_ureg reg)
>  {
>  hppa_tlb_entry *ent = hppa_find_tlb(env, addr);
>  
> -if (unlikely(ent == NULL || ent->entry_valid)) {
> +if (unlikely(ent == NULL)) {
>  qemu_log_mask(LOG_GUEST_ERROR, "ITLBP not following ITLBA\n");
>  return;
>  }
> 

Hmm.  Do you have a broader context for this?  Like maybe the software has just
flushed the entire TLB?  If the entry is valid, and we're not relaxing
permissions, then we might need to flush the softtlb page as well.


r~



[Qemu-devel] [PATCH v2 0/3] nvdimm: clean up

2019-03-11 Thread Wei Yang
Here are three trivial cleanup on argument/variable.

---
v2:
  * remove the last one which breaks hotplug

Wei Yang (3):
  nvdimm: fix typo in nvdimm_build_nvdimm_devices argument
  nvdimm: use *function* directly instead of allocating it again
  nvdimm: use NVDIMM_ACPI_IO_LEN for the proper IO size

 hw/acpi/nvdimm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.19.1




[Qemu-devel] [PATCH v2 2/3] nvdimm: use *function* directly instead of allocating it again

2019-03-11 Thread Wei Yang
At the beginning or nvdimm_build_common_dsm(), variable *function* is
already allocated for Arg2.

This patch reuse variable *function* instead of allocating it again.

Signed-off-by: Wei Yang 
Reviewed-by: Igor Mammedov 
---
 hw/acpi/nvdimm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 39af8cdba8..e63a1ef15d 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -1086,7 +1086,7 @@ static void nvdimm_build_common_dsm(Aml *dev)
  */
 aml_append(method, aml_store(handle, aml_name(NVDIMM_DSM_HANDLE)));
 aml_append(method, aml_store(aml_arg(1), aml_name(NVDIMM_DSM_REVISION)));
-aml_append(method, aml_store(aml_arg(2), aml_name(NVDIMM_DSM_FUNCTION)));
+aml_append(method, aml_store(function, aml_name(NVDIMM_DSM_FUNCTION)));
 
 /*
  * The fourth parameter (Arg3) of _DSM is a package which contains
-- 
2.19.1




[Qemu-devel] [PATCH v2 1/3] nvdimm: fix typo in nvdimm_build_nvdimm_devices argument

2019-03-11 Thread Wei Yang
>From dsm_dma_arrea to dsm_dma_area.

Signed-off-by: Wei Yang 
Reviewed-by: Igor Mammedov 
---
 hw/acpi/nvdimm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e53b2cb681..39af8cdba8 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -1260,7 +1260,7 @@ static void nvdimm_build_nvdimm_devices(Aml *root_dev, 
uint32_t ram_slots)
 }
 
 static void nvdimm_build_ssdt(GArray *table_offsets, GArray *table_data,
-  BIOSLinker *linker, GArray *dsm_dma_arrea,
+  BIOSLinker *linker, GArray *dsm_dma_area,
   uint32_t ram_slots)
 {
 Aml *ssdt, *sb_scope, *dev;
@@ -1307,7 +1307,7 @@ static void nvdimm_build_ssdt(GArray *table_offsets, 
GArray *table_data,
NVDIMM_ACPI_MEM_ADDR);
 
 bios_linker_loader_alloc(linker,
- NVDIMM_DSM_MEM_FILE, dsm_dma_arrea,
+ NVDIMM_DSM_MEM_FILE, dsm_dma_area,
  sizeof(NvdimmDsmIn), false /* high memory */);
 bios_linker_loader_add_pointer(linker,
 ACPI_BUILD_TABLE_FILE, mem_addr_offset, sizeof(uint32_t),
-- 
2.19.1




[Qemu-devel] [PATCH v2 3/3] nvdimm: use NVDIMM_ACPI_IO_LEN for the proper IO size

2019-03-11 Thread Wei Yang
The IO range is defined to 4 bytes with NVDIMM_ACPI_IO_LEN, so it is
more proper to use this macro instead of calculating it by sizeof.

Signed-off-by: Wei Yang 
Reviewed-by: Igor Mammedov 
---
 hw/acpi/nvdimm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e63a1ef15d..2457c1aa44 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -992,7 +992,7 @@ static void nvdimm_build_common_dsm(Aml *dev)
 field = aml_field(NVDIMM_DSM_IOPORT, AML_DWORD_ACC, AML_NOLOCK,
   AML_PRESERVE);
 aml_append(field, aml_named_field(NVDIMM_DSM_NOTIFY,
-   sizeof(uint32_t) * BITS_PER_BYTE));
+   NVDIMM_ACPI_IO_LEN * BITS_PER_BYTE));
 aml_append(method, field);
 
 /*
-- 
2.19.1




[Qemu-devel] [PULL 1/7] hostmem-file: reject invalid pmem file sizes

2019-03-11 Thread Eduardo Habkost
From: Stefan Hajnoczi 

Guests started with NVDIMMs larger than the underlying host file produce
confusing errors inside the guest.  This happens because the guest
accesses pages beyond the end of the file.

Check the pmem file size on startup and print a clear error message if
the size is invalid.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1669053
Cc: Wei Yang 
Cc: Zhang Yi 
Cc: Eduardo Habkost 
Cc: Igor Mammedov 
Signed-off-by: Stefan Hajnoczi 
Message-Id: <20190214031004.32522-3-stefa...@redhat.com>
Reviewed-by: Wei Yang 
Reviewed-by: Igor Mammedov 
Reviewed-by: Pankaj Gupta 
Signed-off-by: Eduardo Habkost 
---
 include/qemu/osdep.h| 13 ++
 backends/hostmem-file.c | 23 ++
 util/oslib-posix.c  | 53 +
 util/oslib-win32.c  |  5 
 4 files changed, 94 insertions(+)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 840af09cb0..303d315c5d 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -570,6 +570,19 @@ void qemu_set_tty_echo(int fd, bool echo);
 void os_mem_prealloc(int fd, char *area, size_t sz, int smp_cpus,
  Error **errp);
 
+/**
+ * qemu_get_pmem_size:
+ * @filename: path to a pmem file
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Determine the size of a persistent memory file.  Besides supporting files on
+ * DAX file systems, this function also supports Linux devdax character
+ * devices.
+ *
+ * Returns: the size or 0 on failure
+ */
+uint64_t qemu_get_pmem_size(const char *filename, Error **errp);
+
 /**
  * qemu_get_pid_name:
  * @pid: pid of a process
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index ce54788048..37ac6445d2 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -56,6 +56,29 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error 
**errp)
 error_setg(errp, "mem-path property not set");
 return;
 }
+
+/*
+ * Verify pmem file size since starting a guest with an incorrect size
+ * leads to confusing failures inside the guest.
+ */
+if (fb->is_pmem) {
+Error *local_err = NULL;
+uint64_t size;
+
+size = qemu_get_pmem_size(fb->mem_path, _err);
+if (!size) {
+error_propagate(errp, local_err);
+return;
+}
+
+if (backend->size > size) {
+error_setg(errp, "size property %" PRIu64 " is larger than "
+   "pmem file \"%s\" size %" PRIu64, backend->size,
+   fb->mem_path, size);
+return;
+}
+}
+
 backend->force_prealloc = mem_prealloc;
 name = host_memory_backend_get_name(backend);
 memory_region_init_ram_from_file(>mr, OBJECT(backend),
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 37c5854b9c..10d90d1783 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -500,6 +500,59 @@ void os_mem_prealloc(int fd, char *area, size_t memory, 
int smp_cpus,
 }
 }
 
+uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
+{
+struct stat st;
+
+if (stat(filename, ) < 0) {
+error_setg(errp, "unable to stat pmem file \"%s\"", filename);
+return 0;
+}
+
+#if defined(__linux__)
+/* Special handling for devdax character devices */
+if (S_ISCHR(st.st_mode)) {
+char *subsystem_path = NULL;
+char *subsystem = NULL;
+char *size_path = NULL;
+char *size_str = NULL;
+uint64_t ret = 0;
+
+subsystem_path = g_strdup_printf("/sys/dev/char/%d:%d/subsystem",
+ major(st.st_rdev), minor(st.st_rdev));
+subsystem = g_file_read_link(subsystem_path, NULL);
+if (!subsystem) {
+error_setg(errp, "unable to read subsystem for pmem file \"%s\"",
+   filename);
+goto devdax_err;
+}
+
+if (!g_str_has_suffix(subsystem, "/dax")) {
+error_setg(errp, "pmem file \"%s\" is not a dax device", filename);
+goto devdax_err;
+}
+
+size_path = g_strdup_printf("/sys/dev/char/%d:%d/size",
+major(st.st_rdev), minor(st.st_rdev));
+if (!g_file_get_contents(size_path, _str, NULL, NULL)) {
+error_setg(errp, "unable to read size for pmem file \"%s\"",
+   size_path);
+goto devdax_err;
+}
+
+ret = g_ascii_strtoull(size_str, NULL, 0);
+
+devdax_err:
+g_free(size_str);
+g_free(size_path);
+g_free(subsystem);
+g_free(subsystem_path);
+return ret;
+}
+#endif /* defined(__linux__) */
+
+return st.st_size;
+}
 
 char *qemu_get_pid_name(pid_t pid)
 {
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index b4c17f5dfa..bd633afab6 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -560,6 +560,11 @@ void os_mem_prealloc(int fd, char *area, size_t memory, 

[Qemu-devel] [PULL 3/7] machine: Move nvdimms state into struct MachineState

2019-03-11 Thread Eduardo Habkost
From: Eric Auger 

As NVDIMM support is looming for ARM and SPAPR, let's
move the acpi_nvdimm_state to the generic machine struct
instead of duplicating the same code in several machines.
It is also renamed into nvdimms_state and becomes a pointer.

nvdimm and nvdimm-persistence become generic machine options.
They become guarded by a nvdimm_supported machine class member.
We also add a description for those options.

Signed-off-by: Eric Auger 
Suggested-by: Igor Mammedov 
Message-Id: <20190308182053.5487-3-eric.au...@redhat.com>
Reviewed-by: Igor Mammedov 
Signed-off-by: Eduardo Habkost 
---
 include/hw/boards.h  |  2 ++
 include/hw/i386/pc.h |  4 ---
 hw/core/machine.c| 65 
 hw/i386/acpi-build.c |  6 ++--
 hw/i386/pc.c | 57 --
 hw/i386/pc_piix.c|  4 +--
 hw/i386/pc_q35.c |  4 +--
 7 files changed, 79 insertions(+), 63 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 9690c71a6d..e231860666 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -210,6 +210,7 @@ struct MachineClass {
  int nb_nodes, ram_addr_t size);
 bool ignore_boot_device_suffixes;
 bool smbus_no_migration_support;
+bool nvdimm_supported;
 
 HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
@@ -272,6 +273,7 @@ struct MachineState {
 const char *cpu_type;
 AccelState *accelerator;
 CPUArchIdList *possible_cpus;
+struct NVDIMMState *nvdimms_state;
 };
 
 #define DEFINE_MACHINE(namestr, machine_initfn) \
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 94fb620d65..263a6343ff 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -45,8 +45,6 @@ struct PCMachineState {
 OnOffAuto vmport;
 OnOffAuto smm;
 
-NVDIMMState acpi_nvdimm_state;
-
 bool acpi_build_enabled;
 bool smbus_enabled;
 bool sata_enabled;
@@ -74,8 +72,6 @@ struct PCMachineState {
 #define PC_MACHINE_MAX_RAM_BELOW_4G "max-ram-below-4g"
 #define PC_MACHINE_VMPORT   "vmport"
 #define PC_MACHINE_SMM  "smm"
-#define PC_MACHINE_NVDIMM   "nvdimm"
-#define PC_MACHINE_NVDIMM_PERSIST   "nvdimm-persistence"
 #define PC_MACHINE_SMBUS"smbus"
 #define PC_MACHINE_SATA "sata"
 #define PC_MACHINE_PIT  "pit"
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 766ca5899d..743fef2898 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -22,6 +22,7 @@
 #include "qemu/error-report.h"
 #include "sysemu/qtest.h"
 #include "hw/pci/pci.h"
+#include "hw/mem/nvdimm.h"
 
 GlobalProperty hw_compat_3_1[] = {
 { "pcie-root-port", "x-speed", "2_5" },
@@ -481,6 +482,47 @@ static void machine_set_memory_encryption(Object *obj, 
const char *value,
 ms->memory_encryption = g_strdup(value);
 }
 
+static bool machine_get_nvdimm(Object *obj, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+return ms->nvdimms_state->is_enabled;
+}
+
+static void machine_set_nvdimm(Object *obj, bool value, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+ms->nvdimms_state->is_enabled = value;
+}
+
+static char *machine_get_nvdimm_persistence(Object *obj, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+return g_strdup(ms->nvdimms_state->persistence_string);
+}
+
+static void machine_set_nvdimm_persistence(Object *obj, const char *value,
+   Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+NVDIMMState *nvdimms_state = ms->nvdimms_state;
+
+if (strcmp(value, "cpu") == 0) {
+nvdimms_state->persistence = 3;
+} else if (strcmp(value, "mem-ctrl") == 0) {
+nvdimms_state->persistence = 2;
+} else {
+error_setg(errp, "-machine nvdimm-persistence=%s: unsupported option",
+   value);
+return;
+}
+
+g_free(nvdimms_state->persistence_string);
+nvdimms_state->persistence_string = g_strdup(value);
+}
+
 void machine_class_allow_dynamic_sysbus_dev(MachineClass *mc, const char *type)
 {
 strList *item = g_new0(strList, 1);
@@ -791,6 +833,28 @@ static void machine_initfn(Object *obj)
 ms->mem_merge = true;
 ms->enable_graphics = true;
 
+if (mc->nvdimm_supported) {
+Object *obj = OBJECT(ms);
+
+ms->nvdimms_state = g_new0(NVDIMMState, 1);
+object_property_add_bool(obj, "nvdimm",
+ machine_get_nvdimm, machine_set_nvdimm,
+ _abort);
+object_property_set_description(obj, "nvdimm",
+"Set on/off to enable/disable "
+"NVDIMM instantiation", NULL);
+
+object_property_add_str(obj, "nvdimm-persistence",
+machine_get_nvdimm_persistence,
+

[Qemu-devel] [Bug 1815889]

2019-03-11 Thread Baker-dylan-c
I'm removing this from the 19.0 blocking tracker. Generally we don't add
bugs to block a release if they were present in the previous release,
additionally there doesn't seem to be any consensus on a solution, at
this moment. If there is a fix implemented I'd be happy to pull that
into a later 19.0 release.

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

Title:
  qemu-system-x86_64 crashed with signal 31 in
  __pthread_setaffinity_new()

Status in Mesa:
  Confirmed
Status in QEMU:
  New
Status in mesa package in Ubuntu:
  Triaged
Status in qemu package in Ubuntu:
  Triaged
Status in mesa source package in Disco:
  Triaged

Bug description:
  Unable to launch Default Fedora 29 images in gnome-boxes

  ProblemType: Crash
  DistroRelease: Ubuntu 19.04
  Package: qemu-system-x86 1:3.1+dfsg-2ubuntu1
  ProcVersionSignature: Ubuntu 4.19.0-12.13-generic 4.19.18
  Uname: Linux 4.19.0-12-generic x86_64
  ApportVersion: 2.20.10-0ubuntu20
  Architecture: amd64
  Date: Thu Feb 14 11:00:45 2019
  ExecutablePath: /usr/bin/qemu-system-x86_64
  KvmCmdLine: COMMAND STAT  EUID  RUID   PID  PPID %CPU COMMAND
  MachineType: Dell Inc. Precision T3610
  ProcEnviron: PATH=(custom, user)
  ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinuz-4.19.0-12-generic 
root=UUID=939b509b-d627-4642-a655-979b44972d17 ro splash quiet vt.handoff=1
  Signal: 31
  SourcePackage: qemu
  StacktraceTop:
   __pthread_setaffinity_new (th=, cpusetsize=128, 
cpuset=0x7f5771fbf680) at ../sysdeps/unix/sysv/linux/pthread_setaffinity.c:34
   () at /usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so
   () at /usr/lib/x86_64-linux-gnu/dri/radeonsi_dri.so
   start_thread (arg=) at pthread_create.c:486
   clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95
  Title: qemu-system-x86_64 crashed with signal 31 in 
__pthread_setaffinity_new()
  UpgradeStatus: Upgraded to disco on 2018-11-14 (91 days ago)
  UserGroups: adm cdrom dip lpadmin plugdev sambashare sudo video
  dmi.bios.date: 11/14/2018
  dmi.bios.vendor: Dell Inc.
  dmi.bios.version: A18
  dmi.board.name: 09M8Y8
  dmi.board.vendor: Dell Inc.
  dmi.board.version: A01
  dmi.chassis.type: 7
  dmi.chassis.vendor: Dell Inc.
  dmi.modalias: 
dmi:bvnDellInc.:bvrA18:bd11/14/2018:svnDellInc.:pnPrecisionT3610:pvr00:rvnDellInc.:rn09M8Y8:rvrA01:cvnDellInc.:ct7:cvr:
  dmi.product.name: Precision T3610
  dmi.product.sku: 05D2
  dmi.product.version: 00
  dmi.sys.vendor: Dell Inc.

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



[Qemu-devel] [PULL 4/7] hostmem-memfd: disable for systems without sealing support

2019-03-11 Thread Eduardo Habkost
From: Ilya Maximets 

If seals are not supported, memfd_create() will fail.
Furthermore, there is no way to disable it in this case because
'.seal' property is not registered.

This issue leads to vhost-user-test failures on RHEL 7.2:

  qemu-system-x86_64: -object memory-backend-memfd,id=mem,size=2M,: \
  failed to create memfd: Invalid argument

and actually breaks the feature on such systems.

Let's restrict memfd backend to systems with sealing support.

Signed-off-by: Ilya Maximets 
Message-Id: <20190311135850.6537-2-i.maxim...@samsung.com>
Signed-off-by: Eduardo Habkost 
---
 backends/hostmem-memfd.c | 18 --
 tests/vhost-user-test.c  |  5 +++--
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index 98c9bf3240..46b15b916a 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -154,15 +154,13 @@ memfd_backend_class_init(ObjectClass *oc, void *data)
   "Huge pages size (ex: 2M, 1G)",
   _abort);
 }
-if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
-object_class_property_add_bool(oc, "seal",
-   memfd_backend_get_seal,
-   memfd_backend_set_seal,
-   _abort);
-object_class_property_set_description(oc, "seal",
-  "Seal growing & shrinking",
-  _abort);
-}
+object_class_property_add_bool(oc, "seal",
+   memfd_backend_get_seal,
+   memfd_backend_set_seal,
+   _abort);
+object_class_property_set_description(oc, "seal",
+  "Seal growing & shrinking",
+  _abort);
 }
 
 static const TypeInfo memfd_backend_info = {
@@ -175,7 +173,7 @@ static const TypeInfo memfd_backend_info = {
 
 static void register_types(void)
 {
-if (qemu_memfd_check(0)) {
+if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
 type_register_static(_backend_info);
 }
 }
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 0c965b3b1e..3817966010 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -178,7 +178,8 @@ static void append_mem_opts(TestServer *server, GString 
*cmd_line,
 int size, enum test_memfd memfd)
 {
 if (memfd == TEST_MEMFD_AUTO) {
-memfd = qemu_memfd_check(0) ? TEST_MEMFD_YES : TEST_MEMFD_NO;
+memfd = qemu_memfd_check(MFD_ALLOW_SEALING) ? TEST_MEMFD_YES
+: TEST_MEMFD_NO;
 }
 
 if (memfd == TEST_MEMFD_YES) {
@@ -930,7 +931,7 @@ static void register_vhost_user_test(void)
  "virtio-net",
  test_read_guest_mem, );
 
-if (qemu_memfd_check(0)) {
+if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
 opts.before = vhost_user_test_setup_memfd;
 qos_add_test("vhost-user/read-guest-mem/memfd",
  "virtio-net",
-- 
2.18.0.rc1.1.g3f1ff2140




[Qemu-devel] [PULL 2/7] nvdimm: Rename AcpiNVDIMMState into NVDIMMState

2019-03-11 Thread Eduardo Habkost
From: Eric Auger 

As we intend to migrate the acpi_nvdimm_state into
the base machine with a new dimms_state name, let's
also rename the datatype.

Signed-off-by: Eric Auger 
Suggested-by: Igor Mammedov 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190308182053.5487-2-eric.au...@redhat.com>
Reviewed-by: Igor Mammedov 
Signed-off-by: Eduardo Habkost 
---
 include/hw/i386/pc.h|  2 +-
 include/hw/mem/nvdimm.h | 10 +-
 hw/acpi/nvdimm.c| 18 +-
 hw/i386/pc.c|  2 +-
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 54222a202d..94fb620d65 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -45,7 +45,7 @@ struct PCMachineState {
 OnOffAuto vmport;
 OnOffAuto smm;
 
-AcpiNVDIMMState acpi_nvdimm_state;
+NVDIMMState acpi_nvdimm_state;
 
 bool acpi_build_enabled;
 bool smbus_enabled;
diff --git a/include/hw/mem/nvdimm.h b/include/hw/mem/nvdimm.h
index c5c9b3c7f8..523a9b3d4a 100644
--- a/include/hw/mem/nvdimm.h
+++ b/include/hw/mem/nvdimm.h
@@ -123,7 +123,7 @@ struct NvdimmFitBuffer {
 };
 typedef struct NvdimmFitBuffer NvdimmFitBuffer;
 
-struct AcpiNVDIMMState {
+struct NVDIMMState {
 /* detect if NVDIMM support is enabled. */
 bool is_enabled;
 
@@ -141,13 +141,13 @@ struct AcpiNVDIMMState {
 int32_t persistence;
 char*persistence_string;
 };
-typedef struct AcpiNVDIMMState AcpiNVDIMMState;
+typedef struct NVDIMMState NVDIMMState;
 
-void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
+void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
 FWCfgState *fw_cfg, Object *owner);
 void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
-   BIOSLinker *linker, AcpiNVDIMMState *state,
+   BIOSLinker *linker, NVDIMMState *state,
uint32_t ram_slots);
-void nvdimm_plug(AcpiNVDIMMState *state);
+void nvdimm_plug(NVDIMMState *state);
 void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev);
 #endif
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index e53b2cb681..f73cfb9d90 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -382,7 +382,7 @@ nvdimm_build_structure_caps(GArray *structures, uint32_t 
capabilities)
 nfit_caps->capabilities = cpu_to_le32(capabilities);
 }
 
-static GArray *nvdimm_build_device_structure(AcpiNVDIMMState *state)
+static GArray *nvdimm_build_device_structure(NVDIMMState *state)
 {
 GSList *device_list = nvdimm_get_device_list();
 GArray *structures = g_array_new(false, true /* clear */, 1);
@@ -416,7 +416,7 @@ static void nvdimm_init_fit_buffer(NvdimmFitBuffer *fit_buf)
 fit_buf->fit = g_array_new(false, true /* clear */, 1);
 }
 
-static void nvdimm_build_fit_buffer(AcpiNVDIMMState *state)
+static void nvdimm_build_fit_buffer(NVDIMMState *state)
 {
 NvdimmFitBuffer *fit_buf = >fit_buf;
 
@@ -425,12 +425,12 @@ static void nvdimm_build_fit_buffer(AcpiNVDIMMState 
*state)
 fit_buf->dirty = true;
 }
 
-void nvdimm_plug(AcpiNVDIMMState *state)
+void nvdimm_plug(NVDIMMState *state)
 {
 nvdimm_build_fit_buffer(state);
 }
 
-static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets,
+static void nvdimm_build_nfit(NVDIMMState *state, GArray *table_offsets,
   GArray *table_data, BIOSLinker *linker)
 {
 NvdimmFitBuffer *fit_buf = >fit_buf;
@@ -570,7 +570,7 @@ nvdimm_dsm_no_payload(uint32_t func_ret_status, hwaddr 
dsm_mem_addr)
 #define NVDIMM_QEMU_RSVD_HANDLE_ROOT 0x1
 
 /* Read FIT data, defined in docs/specs/acpi_nvdimm.txt. */
-static void nvdimm_dsm_func_read_fit(AcpiNVDIMMState *state, NvdimmDsmIn *in,
+static void nvdimm_dsm_func_read_fit(NVDIMMState *state, NvdimmDsmIn *in,
  hwaddr dsm_mem_addr)
 {
 NvdimmFitBuffer *fit_buf = >fit_buf;
@@ -619,7 +619,7 @@ exit:
 }
 
 static void
-nvdimm_dsm_handle_reserved_root_method(AcpiNVDIMMState *state,
+nvdimm_dsm_handle_reserved_root_method(NVDIMMState *state,
NvdimmDsmIn *in, hwaddr dsm_mem_addr)
 {
 switch (in->function) {
@@ -863,7 +863,7 @@ nvdimm_dsm_read(void *opaque, hwaddr addr, unsigned size)
 static void
 nvdimm_dsm_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
 {
-AcpiNVDIMMState *state = opaque;
+NVDIMMState *state = opaque;
 NvdimmDsmIn *in;
 hwaddr dsm_mem_addr = val;
 
@@ -925,7 +925,7 @@ void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, 
DeviceState *dev)
 }
 }
 
-void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
+void nvdimm_init_acpi_state(NVDIMMState *state, MemoryRegion *io,
 FWCfgState *fw_cfg, Object *owner)
 {
 memory_region_init_io(>io_mr, owner, _dsm_ops, state,
@@ -1319,7 +1319,7 @@ static void nvdimm_build_ssdt(GArray *table_offsets, 
GArray 

[Qemu-devel] [PULL 5/7] memfd: always check for MFD_CLOEXEC

2019-03-11 Thread Eduardo Habkost
From: Ilya Maximets 

QEMU always sets this flag unconditionally. We need to
check if it's supported.

Signed-off-by: Ilya Maximets 
Reviewed-by: Marc-André Lureau 
Message-Id: <20190311135850.6537-3-i.maxim...@samsung.com>
Signed-off-by: Eduardo Habkost 
---
 util/memfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/memfd.c b/util/memfd.c
index 8debd0d037..d74ce4d793 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -188,7 +188,7 @@ bool qemu_memfd_alloc_check(void)
 bool qemu_memfd_check(unsigned int flags)
 {
 #ifdef CONFIG_LINUX
-int mfd = memfd_create("test", flags);
+int mfd = memfd_create("test", flags | MFD_CLOEXEC);
 
 if (mfd >= 0) {
 close(mfd);
-- 
2.18.0.rc1.1.g3f1ff2140




[Qemu-devel] [PULL 0/7] Machine queue, 2019-03-11

2019-03-11 Thread Eduardo Habkost
The following changes since commit 336cfef495f0cd5b1606251c52628d0372e9a809:

  Makefile: Don't install non-sphinx files in sphinx docs install (2019-03-11 
11:10:44 +)

are available in the Git repository at:

  git://github.com/ehabkost/qemu.git tags/machine-next-pull-request

for you to fetch changes up to edaed6c711f07267785a05a633d97dc9268a7385:

  memfd: improve error messages (2019-03-11 17:17:01 -0300)


Machine queue, 2019-03-11

* memfd fixes (Ilya Maximets)
* Move nvdimms state into struct MachineState (Eric Auger)
* hostmem-file: reject invalid pmem file sizes (Stefan Hajnoczi)



Queue for Machine Core patches


Eric Auger (2):
  nvdimm: Rename AcpiNVDIMMState into NVDIMMState
  machine: Move nvdimms state into struct MachineState

Ilya Maximets (4):
  hostmem-memfd: disable for systems without sealing support
  memfd: always check for MFD_CLOEXEC
  memfd: set up correct errno if not supported
  memfd: improve error messages

Stefan Hajnoczi (1):
  hostmem-file: reject invalid pmem file sizes

 include/hw/boards.h  |  2 ++
 include/hw/i386/pc.h |  4 ---
 include/hw/mem/nvdimm.h  | 10 +++
 include/qemu/osdep.h | 13 
 backends/hostmem-file.c  | 23 ++
 backends/hostmem-memfd.c | 18 +--
 hw/acpi/nvdimm.c | 18 +--
 hw/core/machine.c| 65 
 hw/i386/acpi-build.c |  6 ++--
 hw/i386/pc.c | 57 ---
 hw/i386/pc_piix.c|  4 +--
 hw/i386/pc_q35.c |  4 +--
 tests/vhost-user-test.c  |  5 ++--
 util/memfd.c | 10 +--
 util/oslib-posix.c   | 53 
 util/oslib-win32.c   |  5 
 16 files changed, 206 insertions(+), 91 deletions(-)

-- 
2.18.0.rc1.1.g3f1ff2140




[Qemu-devel] [PULL 6/7] memfd: set up correct errno if not supported

2019-03-11 Thread Eduardo Habkost
From: Ilya Maximets 

qemu_memfd_create() prints the value of 'errno' which is not
set in this case.

Signed-off-by: Ilya Maximets 
Reviewed-by: Marc-André Lureau 
Message-Id: <20190311135850.6537-4-i.maxim...@samsung.com>
Signed-off-by: Eduardo Habkost 
---
 util/memfd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/memfd.c b/util/memfd.c
index d74ce4d793..393d23da96 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -40,6 +40,7 @@ static int memfd_create(const char *name, unsigned int flags)
 #ifdef __NR_memfd_create
 return syscall(__NR_memfd_create, name, flags);
 #else
+errno = ENOSYS;
 return -1;
 #endif
 }
-- 
2.18.0.rc1.1.g3f1ff2140




Re: [Qemu-devel] [PATCH v3 15/20] Boot Linux Console Test: add a test for mips64el + malta

2019-03-11 Thread Cleber Rosa
On Sat, Mar 02, 2019 at 10:39:47PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/21/19 1:57 AM, Cleber Rosa wrote:
> > +import os
> 
> One import is enough :P
>

Oh my... thanks for spotting this.

- Cleber.



[Qemu-devel] [PULL 7/7] memfd: improve error messages

2019-03-11 Thread Eduardo Habkost
From: Ilya Maximets 

This gives more information about the failure.
Additionally 'ENOSYS' returned for a non-Linux platforms instead of
'errno', which is not initilaized in this case.

Signed-off-by: Ilya Maximets 
Reviewed-by: Marc-André Lureau 
Message-Id: <20190311135850.6537-5-i.maxim...@samsung.com>
Signed-off-by: Eduardo Habkost 
---
 util/memfd.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/util/memfd.c b/util/memfd.c
index 393d23da96..00334e5b21 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -71,14 +71,18 @@ int qemu_memfd_create(const char *name, size_t size, bool 
hugetlb,
 }
 mfd = memfd_create(name, flags);
 if (mfd < 0) {
+error_setg_errno(errp, errno,
+ "failed to create memfd with flags 0x%x", flags);
 goto err;
 }
 
 if (ftruncate(mfd, size) == -1) {
+error_setg_errno(errp, errno, "failed to resize memfd to %zu", size);
 goto err;
 }
 
 if (seals && fcntl(mfd, F_ADD_SEALS, seals) == -1) {
+error_setg_errno(errp, errno, "failed to add seals 0x%x", seals);
 goto err;
 }
 
@@ -88,8 +92,9 @@ err:
 if (mfd >= 0) {
 close(mfd);
 }
+#else
+error_setg_errno(errp, ENOSYS, "failed to create memfd");
 #endif
-error_setg_errno(errp, errno, "failed to create memfd");
 return -1;
 }
 
-- 
2.18.0.rc1.1.g3f1ff2140




Re: [Qemu-devel] [PATCH v3 13/20] scripts/qemu.py: support adding a console with the default serial device

2019-03-11 Thread Cleber Rosa
On Fri, Mar 01, 2019 at 11:55:13AM +0100, Cornelia Huck wrote:
> On Wed, 20 Feb 2019 19:57:46 -0500
> Cleber Rosa  wrote:
> 
> > The set_console() utility function traditionally adds a device either
> > based on the explicitly given device type, or based on the machine type,
> > a known good type of device.
> 
> Hm, I find this sentence hard to parse... maybe it should be "either
> adds a device..." and "adds a known good type of device"?
>

OK, sounds good, but I may change it a little bit given that you
following feedback made me change (positively) a lot here.

> > 
> > But, for a number of machine types, it may be impossible or
> > inconvenient to add the devices my means of "-device" command line
> 
> s/my/by/
>

Same here.

> > options, and then it may better to just use the "-serial" option and
> > let QEMU itself, based on the machine type, set the device
> > accordingly.
> > 
> > To achieve that, the behavior of set_console() now flags the intention
> > to add a console device on launch(), and if no explicit device type is
> > given, and there's no definition on CONSOLE_DEV_TYPES, the "-serial"
> > is going to be added to the QEMU command line, instead of raising
> > exceptions.
> > 
> > Based on testing with different machine types, the CONSOLE_DEV_TYPES
> > is now being set to the bare essential entries (one entry to be
> > honest), for machine types that can not easily give us a working
> > console with "-serial".
> > 
> > Signed-off-by: Cleber Rosa 
> > ---
> >  scripts/qemu.py | 39 +++
> >  1 file changed, 19 insertions(+), 20 deletions(-)
> > 
> > diff --git a/scripts/qemu.py b/scripts/qemu.py
> > index ee85309923..bd1d2e2b9a 100644
> > --- a/scripts/qemu.py
> > +++ b/scripts/qemu.py
> > @@ -42,11 +42,6 @@ def kvm_available(target_arch=None):
> >  
> >  #: Maps machine types to the preferred console device types
> >  CONSOLE_DEV_TYPES = {
> > -r'^clipper$': 'isa-serial',
> > -r'^malta': 'isa-serial',
> > -r'^(pc.*|q35.*|isapc)$': 'isa-serial',
> > -r'^(40p|powernv|prep)$': 'isa-serial',
> > -r'^pseries.*': 'spapr-vty',
> >  r'^s390-ccw-virtio.*': 'sclpconsole',
> 
> FWIW, s390x has supported the '-serial' parameter since 3.0 (commit
> 052888f043bacb18231046045b1f9cd946703170). Maybe you can drop this now?

Yes, thanks for pointing that out!  I'm simplifying the set_console()
behavior and dropping the CONSOLE_DEV_TYPES dict completely.

> If not, what error are you getting?

No errors, it seems to work great now.

- Cleber.



Re: [Qemu-devel] [PATCH] tests: test-bdrv-graph-mod: fix memory leak

2019-03-11 Thread Li Qiang
Philippe Mathieu-Daudé  于2019年3月10日周日 下午10:34写道:

> On 3/10/19 12:34 PM, Li Qiang wrote:
> > Fixes: 2dbfadf
>
>   ^ Please keep tags together (with Signed-off-by, ...)
>
> > Spotted by ASAN when 'make check'.
>
> I'm not native English speaker but I'd say:
>
>

Thanks your advice.
Hope the maintainer will do this minor adjustment.

Thanks,
Li Qiang




> Spotted by ASAN [with] 'make check'.
>
> or
>
> Spotted by ASAN [while running] 'make check'.
>
> Here goes:
>
> "Fixes: 2dbfadf"
>
> > Signed-off-by: Li Qiang 
> > ---
> >  tests/test-bdrv-graph-mod.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c
> > index 458dfa6661..8bf0fe735d 100644
> > --- a/tests/test-bdrv-graph-mod.c
> > +++ b/tests/test-bdrv-graph-mod.c
> > @@ -117,6 +117,7 @@ static void test_update_perm_tree(void)
> >
> >  bdrv_unref(bs);
> >  blk_unref(root);
> > +error_free(local_err);
>
> Reviewed-by: Philippe Mathieu-Daudé 
>
> >  }
> >
> >  /*
> >
>


Re: [Qemu-devel] [PATCH 06/11] target/hppa: ignore DIAG opcode

2019-03-11 Thread Richard Henderson
On 3/11/19 12:15 PM, Sven Schnelle wrote:
> +static bool trans_diag(DisasContext *ctx, arg_diag *a)
> +{
> +qemu_log_mask(LOG_UNIMP, "DIAG opcode ignored\n");
> +return true;

This needs to free the nullify condition, as with trans_nop.
I've fixed this up while applying.


r~



Re: [Qemu-devel] [PATCH v3 11/20] Boot Linux Console Test: increase timeout

2019-03-11 Thread Cleber Rosa
On Sat, Mar 02, 2019 at 10:43:52PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/21/19 1:57 AM, Cleber Rosa wrote:
> > When running on very low powered environments, some tests may time out
> > causing false negatives.  As a conservative change, and for
> > considering that human time (investigating false negatives) is worth
> > more than some extra machine cycles (and time), let's increase the
> > overall timeout.
> 
> However this is annoying when a test is stuck and you work on battery...
>

I agree, but at this point, the more we try to fix all use cases, the
furthest we get to something reasonable IMO.

Any ideas on how to do better here while still keeping things simple?

- Cleber.

> > 
> > CC: Alex Bennée 
> > Signed-off-by: Cleber Rosa 
> > ---
> >  tests/acceptance/boot_linux_console.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/acceptance/boot_linux_console.py 
> > b/tests/acceptance/boot_linux_console.py
> > index cc5dcd7373..fa721a7355 100644
> > --- a/tests/acceptance/boot_linux_console.py
> > +++ b/tests/acceptance/boot_linux_console.py
> > @@ -21,7 +21,7 @@ class BootLinuxConsole(Test):
> >  :avocado: enable
> >  """
> >  
> > -timeout = 60
> > +timeout = 90
> >  
> >  KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
> >  
> > 



Re: [Qemu-devel] [PATCH v3 09/20] Boot Linux Console Test: update the x86_64 kernel

2019-03-11 Thread Cleber Rosa
On Sat, Mar 02, 2019 at 10:42:13PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/21/19 1:57 AM, Cleber Rosa wrote:
> > To the stock Fedora 29 kernel, from the Fedora 28.  New tests will be
> 
> [Update] to ... ?
>

Right... I used this line as a continuation of the commit title,
but probably shouldn't have done that.

> > added using the 29 kernel, so for consistency, let's also update it
> > here.
> > 
> > Signed-off-by: Cleber Rosa 
> > Reviewed-by: Caio Carrara 
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Tested-by: Philippe Mathieu-Daudé 
>

Thanks!
- Cleber.

> > ---
> >  tests/acceptance/boot_linux_console.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/acceptance/boot_linux_console.py 
> > b/tests/acceptance/boot_linux_console.py
> > index 89df7f6e4f..35b31162d4 100644
> > --- a/tests/acceptance/boot_linux_console.py
> > +++ b/tests/acceptance/boot_linux_console.py
> > @@ -28,9 +28,9 @@ class BootLinuxConsole(Test):
> >  :avocado: tags=arch:x86_64
> >  :avocado: tags=machine:pc
> >  """
> > -kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
> > +kernel_url = ('https://mirrors.kernel.org/fedora/releases/29/'
> >'Everything/x86_64/os/images/pxeboot/vmlinuz')
> > -kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
> > +kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
> >  kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> >  
> >  self.vm.set_machine('pc')
> > 



Re: [Qemu-devel] [PATCH v3 09/20] Boot Linux Console Test: update the x86_64 kernel

2019-03-11 Thread Cleber Rosa
On Fri, Mar 01, 2019 at 11:41:44AM +0100, Cornelia Huck wrote:
> On Wed, 20 Feb 2019 19:57:42 -0500
> Cleber Rosa  wrote:
> 
> > To the stock Fedora 29 kernel, from the Fedora 28.  New tests will be
> > added using the 29 kernel, so for consistency, let's also update it
> > here.
> > 
> > Signed-off-by: Cleber Rosa 
> > Reviewed-by: Caio Carrara 
> > ---
> >  tests/acceptance/boot_linux_console.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tests/acceptance/boot_linux_console.py 
> > b/tests/acceptance/boot_linux_console.py
> > index 89df7f6e4f..35b31162d4 100644
> > --- a/tests/acceptance/boot_linux_console.py
> > +++ b/tests/acceptance/boot_linux_console.py
> > @@ -28,9 +28,9 @@ class BootLinuxConsole(Test):
> >  :avocado: tags=arch:x86_64
> >  :avocado: tags=machine:pc
> >  """
> > -kernel_url = ('https://mirrors.kernel.org/fedora/releases/28/'
> > +kernel_url = ('https://mirrors.kernel.org/fedora/releases/29/'
> >'Everything/x86_64/os/images/pxeboot/vmlinuz')
> > -kernel_hash = '238e083e114c48200f80d889f7e32eeb2793e02a'
> > +kernel_hash = '23bebd2680757891cf7adedb033532163a792495'
> >  kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> >  
> >  self.vm.set_machine('pc')
> 
> This looks reasonable; but an obvious follow-up question is how we
> handle updates to new distro levels in general.
> 
> I don't think we'll need to run acceptance tests on old qemus, though.

No, I don't think we have to bother with that.  Backport(er)s would be
expected to handle this (if they ever come to existence).

You do have a point about consistenly updating those images, though.
I won't pretend to have an answer for that just yet... but it's
possible that these "kernel_urls" can be turned into parameters
that will eventually be expand based on the user's selected distro
and version (similar to the Avocado vmimage library[1]).

- Cleber.

[1] 
https://avocado-framework.readthedocs.io/en/69.0/api/utils/avocado.utils.html#avocado.utils.vmimage.get



Re: [Qemu-devel] [PATCH v3 07/20] Acceptance tests: look for target architecture in test tags first

2019-03-11 Thread Cleber Rosa
On Fri, Mar 01, 2019 at 11:37:19AM +0100, Cornelia Huck wrote:
> On Wed, 20 Feb 2019 19:57:40 -0500
> Cleber Rosa  wrote:
> 
> > A test can, optionally, be tagged for one or many architectures.  If a
> > test has been tagged for a single architecture, there's a high chance
> > that the test won't run on other architectures.  This changes the
> > default order of choosing a default target architecture to use based
> > on the 'arch' tag value first.
> > 
> > The precedence order is for choosing a QEMU binary to use for a test
> > is now:
> > 
> >  * qemu_bin parameter
> >  * arch parameter
> >  * arch tag value (for example, x86_64 if ":avocado: tags=arch:x86_64
> >is used)
> > 
> > This means that if one runs:
> > 
> >  $ avocado run -p qemu_bin=/usr/bin/qemu-system-x86_64 test.py
> > 
> > No arch parameter or tag will influence the selection of the QEMU
> > target binary.  If one runs:
> > 
> >  $ avocado run -p arch=ppc64 test.py
> > 
> > The target binary selection mechanism will attempt to find a binary
> > such as "ppc64-softmmu/qemu-system-ppc64".  And finally, if one runs
> > a test that is tagged (in its docstring) with "arch:aarch64":
> > 
> >  $ avocado run aarch64.py
> > 
> > The target binary selection mechanism will attempt to find a binary
> > such as "aarch64-softmmu/qemu-system-aarch64".
> > 
> > At this time, no provision is made to cancel the execution of tests if
> > the arch parameter given (manually) does not match the test "arch"
> > tag, but it may be a useful default behavior to be added in the
> > future.
> 
> Would it be useful as well to cancel a test if the passed-in qemu_bin
> parameter clashed with the tagged architecture?
> 

It would... in fact, a much older version of this proposal did that.
I'm waiting for the most basic things to get in, before adding yet
another "special" behavior.

Thanks for the review and suggestion.
- Cleber.

> > 
> > Signed-off-by: Cleber Rosa 
> > ---
> >  docs/devel/testing.rst| 4 +++-
> >  tests/acceptance/avocado_qemu/__init__.py | 7 ++-
> >  2 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> > index 6035db1b44..87bcf8ef43 100644
> > --- a/docs/devel/testing.rst
> > +++ b/docs/devel/testing.rst
> > @@ -702,7 +702,9 @@ A test may, for instance, use the same value when 
> > selecting the
> >  architecture of a kernel or disk image to boot a VM with.
> >  
> >  The ``arch`` attribute will be set to the test parameter of the same
> > -name, and if one is not given explicitly, it will be set to ``None``.
> > +name.  If one is not given explicitly, it will either be set to
> > +``None``, or, if the test is tagged with one (and only one)
> > +``:avocado: tags=arch:VALUE`` tag, it will be set to ``VALUE``.
> >  
> >  qemu_bin
> >  
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py 
> > b/tests/acceptance/avocado_qemu/__init__.py
> > index f580582602..9e98d113cb 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -53,7 +53,12 @@ def pick_default_qemu_bin(arch=None):
> >  class Test(avocado.Test):
> >  def setUp(self):
> >  self.vm = None
> > -self.arch = self.params.get('arch')
> > +arches = self.tags.get('arch', [])
> > +if len(arches) == 1:
> > +arch = arches.pop()
> > +else:
> > +arch = None
> > +self.arch = self.params.get('arch', default=arch)
> >  default_qemu_bin = pick_default_qemu_bin(arch=self.arch)
> >  self.qemu_bin = self.params.get('qemu_bin',
> >  default=default_qemu_bin)
> 
> Reviewed-by: Cornelia Huck 



Re: [Qemu-devel] [PATCH v3 05/20] Acceptance tests: introduce arch parameter and attribute

2019-03-11 Thread Cleber Rosa
On Fri, Mar 01, 2019 at 11:27:10AM +0100, Cornelia Huck wrote:
> On Wed, 20 Feb 2019 19:57:38 -0500
> Cleber Rosa  wrote:
> 
> > It's useful to define the architecture that should be used in
> > situations such as:
> >  * the intended target of the QEMU binary to be used on tests
> >  * the architecture of code to be run within the QEMU binary, such
> >as a kernel image or a full blown guest OS image
> > 
> > This commit introduces both a test parameter and a test instance
> > attribute, that will contain such a value.
> > 
> > Now, when the "arch" test parameter is given, it will influence the
> > selection of the default QEMU binary, if one is not given explicitly
> > by means of the "qemu_img" parameter.
> > 
> > Signed-off-by: Cleber Rosa 
> > Reviewed-by: Philippe Mathieu-Daudé 
> > ---
> >  docs/devel/testing.rst| 28 +++
> >  tests/acceptance/avocado_qemu/__init__.py | 14 +---
> >  2 files changed, 39 insertions(+), 3 deletions(-)
> > 
> > diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> > index ceaaafc69f..6035db1b44 100644
> > --- a/docs/devel/testing.rst
> > +++ b/docs/devel/testing.rst
> > @@ -689,6 +689,21 @@ vm
> >  A QEMUMachine instance, initially configured according to the given
> >  ``qemu_bin`` parameter.
> >  
> > +arch
> > +
> > +
> > +The architecture can be used on different levels of the stack, e.g. by
> > +the framework or by the test itself.  At the framework level, it will
> > +will currently influence the selection of a QEMU binary (when one is
> 
> s/will will/will/

Thanks for spotting this!

> 
> > +not explicitly given).
> > +
> > +Tests are also free to use this attribute value, for their own needs.
> > +A test may, for instance, use the same value when selecting the
> > +architecture of a kernel or disk image to boot a VM with.
> > +
> > +The ``arch`` attribute will be set to the test parameter of the same
> > +name, and if one is not given explicitly, it will be set to ``None``.
> > +
> >  qemu_bin
> >  
> >  
> > @@ -711,6 +726,19 @@ like the following:
> >  
> >PARAMS (key=qemu_bin, path=*, default=x86_64-softmmu/qemu-system-x86_64) 
> > => 'x86_64-softmmu/qemu-system-x86_64
> >  
> > +arch
> > +
> > +
> > +The architecture that will influence the selection of a QEMU binary
> > +(when one is not explicitly given).
> > +
> > +Tests are also free to use this parameter value, for their own needs.
> > +A test may, for instance, use the same value when selecting the
> > +architecture of a kernel or disk image to boot a VM with.
> > +
> > +This parameter has a direct relation with the ``arch`` attribute.  If
> > +not given, it will default to None.
> > +
> >  qemu_bin
> >  
> >  
> > diff --git a/tests/acceptance/avocado_qemu/__init__.py 
> > b/tests/acceptance/avocado_qemu/__init__.py
> > index d8d5b48dac..f580582602 100644
> > --- a/tests/acceptance/avocado_qemu/__init__.py
> > +++ b/tests/acceptance/avocado_qemu/__init__.py
> > @@ -23,16 +23,22 @@ def is_readable_executable_file(path):
> >  return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
> >  
> >  
> > -def pick_default_qemu_bin():
> > +def pick_default_qemu_bin(arch=None):
> >  """
> >  Picks the path of a QEMU binary, starting either in the current working
> >  directory or in the source tree root directory.
> >  
> > +:param arch: the arch to use when looking for a QEMU binary (the target
> > + will match the arch given).  If None (the default) arch
> 
> s/None (the default)/None (the default),/ makes that sentence less
> confusing :)
> 

Good point, thanks!

- Cleber.



Re: [Qemu-devel] [PATCH v3 06/20] Acceptance tests: use "arch:" tag to filter target specific tests

2019-03-11 Thread Cleber Rosa
On Fri, Mar 01, 2019 at 11:32:27AM +0100, Cornelia Huck wrote:
> On Wed, 20 Feb 2019 19:57:39 -0500
> Cleber Rosa  wrote:
> 
> > Currently, the only test that contains some target architecture
> > information is "boot_linux_console.py" which test contains a "x86_64"
> 
> But there are two others changed by you here, aren't there?
>

Yes, the "only test" is now outdated.  Fixing it.

> > tag.  But that tag is not respected in the default execution, that is,
> > "make check-acceptance" doesn't do anything with it.
> > 
> > That said, even the target architecture handling currently present in
> > the "avocado_qemu.Test" class, class is pretty limited.  For instance,
> 
> s/class, class/class/

You are so good at proof reading! Kudos!

> 
> > by default, it chooses a target based on the host architecture.
> > 
> > Because the original implementation of the tags feature in Avocado did
> > not include any time of namespace or "key:val" mechanism, no tag has
> > relation to another tag.  The new implementation of the tags feature
> > from version 67.0 onwards, allows "key:val" tags, and because of that,
> > a test can be classified with a tag in a given key.  For instance, the
> > new proposed version of the "boot_linux_console.py" test, which
> > downloads and attempts to run a x86_64 kernel, is now tagged as:
> > 
> >   :avocado: tags=arch:x86_64
> > 
> > This means that it can be filtered (out) when no x86_64 target is
> > available.  At the same time, tests that don't have a "arch:" tag,
> > will not be filtered out.
> > 
> > Signed-off-by: Cleber Rosa 
> > ---
> >  tests/Makefile.include | 3 +++
> >  tests/acceptance/boot_linux_console.py | 2 +-
> >  tests/acceptance/linux_initrd.py   | 2 +-
> >  tests/acceptance/virtio_version.py | 2 +-
> >  tests/requirements.txt | 2 +-
> >  5 files changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 93ea42553e..633992603d 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -1090,6 +1090,7 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
> >  # Any number of command separated loggers are accepted.  For more
> >  # information please refer to "avocado --help".
> >  AVOCADO_SHOW=app
> > +AVOCADO_TAGS=$(patsubst %-softmmu,-t arch:%, $(filter 
> > %-softmmu,$(TARGET_DIRS)))
> >  
> >  ifneq ($(findstring v2,"v$(PYTHON_VERSION)"),v2)
> >  $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
> > @@ -1115,6 +1116,8 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR)
> > $(call quiet-command, \
> >  $(TESTS_VENV_DIR)/bin/python -m avocado \
> >  --show=$(AVOCADO_SHOW) run 
> > --job-results-dir=$(TESTS_RESULTS_DIR) \
> > +--filter-by-tags-include-empty 
> > --filter-by-tags-include-empty-key \
> > +$(AVOCADO_TAGS) \
> >  --failfast=on $(SRC_PATH)/tests/acceptance, \
> >  "AVOCADO", "tests/acceptance")
> >  
> > diff --git a/tests/acceptance/boot_linux_console.py 
> > b/tests/acceptance/boot_linux_console.py
> > index 98324f7591..46b20bdfe2 100644
> > --- a/tests/acceptance/boot_linux_console.py
> > +++ b/tests/acceptance/boot_linux_console.py
> > @@ -19,7 +19,7 @@ class BootLinuxConsole(Test):
> >  and the kernel command line is properly passed from QEMU to the kernel
> >  
> >  :avocado: enable
> > -:avocado: tags=x86_64
> > +:avocado: tags=arch:x86_64
> >  """
> >  
> >  timeout = 60
> > diff --git a/tests/acceptance/linux_initrd.py 
> > b/tests/acceptance/linux_initrd.py
> > index 737355c2ef..c75e29be70 100644
> > --- a/tests/acceptance/linux_initrd.py
> > +++ b/tests/acceptance/linux_initrd.py
> > @@ -19,7 +19,7 @@ class LinuxInitrd(Test):
> >  Checks QEMU evaluates correctly the initrd file passed as -initrd 
> > option.
> >  
> >  :avocado: enable
> > -:avocado: tags=x86_64
> > +:avocado: tags=arch:x86_64
> >  """
> >  
> >  timeout = 60
> > diff --git a/tests/acceptance/virtio_version.py 
> > b/tests/acceptance/virtio_version.py
> > index ce990250d8..3b280e7fc3 100644
> > --- a/tests/acceptance/virtio_version.py
> > +++ b/tests/acceptance/virtio_version.py
> > @@ -62,7 +62,7 @@ class VirtioVersionCheck(Test):
> >  `disable-legacy`.
> >  
> >  :avocado: enable
> > -:avocado: tags=x86_64
> > +:avocado: tags=arch:x86_64
> >  """
> >  
> >  # just in case there are failures, show larger diff:
> > diff --git a/tests/requirements.txt b/tests/requirements.txt
> > index 64c6e27a94..002ded6a22 100644
> > --- a/tests/requirements.txt
> > +++ b/tests/requirements.txt
> > @@ -1,4 +1,4 @@
> >  # Add Python module requirements, one per line, to be installed
> >  # in the tests/venv Python virtual environment. For more info,
> >  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> > -avocado-framework==65.0
> > +avocado-framework==68.0
> 
> I think you should explain why you bump the required version to 68.0
> 

Re: [Qemu-devel] [PATCH] usb-mtp: fix return status of delete

2019-03-11 Thread Bandan Das
Peter Maydell  writes:
...
>> Ok, this is easier. Now, I know what you are referring to
>> instead of guessing what and how I should be explainng.
>>
>> What you said is essentially correct. When deleting a
>> single object that's a file, the return value would either
>> be OK or STORE_READ_ONLY.
>>
>> When deleting an object which is a folder, Qemu goes through
>> its contents. If it succeeds in deleting all the content objects,
>> the result is success. If one or some objects couldn't be deleted for
>> whatever reason, the result is RES_PARTIAL_DELETE and if none
>> of the objects could be deleted, Qemu returns a READ_ONLY.
>>
>> In usb_mtp_object_delete(), based on the return value of this
>> function, we set s->result appropriately.
>
> OK. So we can do this with a return value where the
> two relevant bits indicate:
>  * bit 0: We had at least one successful deletion
>  * bit 1: We had at least one failed deletion
>
> and then the correct RES value is:
>  * only bit 0 set: READ_ONLY
>  * only bit 1 set: OK
>  * both bits set: PARTIAL_DELETE
>  * neither bit set: can't happen
>
> This is what your patch does, I think, but you've named
> the "at least one deletion succeeded" bit "ALL_DELETE"
> (even though it can be set in a return value where not
> all of the deletions succeeded) and the "at least one
> deletion failed" bit "READ_ONLY" (even though it can
> be set in a return value where some deletions succeeded),
> which is what I found confusing.
>
> I think the code would be easier to understand if we:
>  * picked clearer names for the bits, like
>DELETE_SUCCESS and DELETE_FAILURE
>  * had a comment explaining what the return value
>of the function means, something like:
>
> /*
>  * Delete the object @o and all its children. In the
>  * return value, the DELETE_SUCCESS bit is set if at
>  * least one of the deletions succeeded, and the
>  * DELETE_FAILURE bit is set if at least one of the
>  * deletions failed. If the tree of objects was only
>  * partially deleted then both bits will be set.
>  */
>
> But really the second of these is the more important:
> slightly confusing naming is OK if there is a good
> comment explaining what is going on (and my suggested
> bit flag names don't really stand on their own without
> any explanation either). So if you strongly prefer your names
> for the bits that's ok, but please do add a comment,
> either at the top of the function or documenting
> the meaning of the enum values.
>
Peter, thank you for the thorough review, I really appreciate it.
I definitely want to make this code less confusing to the next
potential reviewer. I will address all your suggestions in the
next version of the patch.

Bandan

> thanks
> -- PMM



[Qemu-devel] [PATCH v5 2/2] spapr-rtas: add ibm, get-vpd RTAS interface

2019-03-11 Thread Maxiwell S. Garcia
This adds a handler for ibm,get-vpd RTAS calls, allowing pseries guest
to collect host information. It is disabled by default to avoid unwanted
information leakage. To enable it, use:

‘-M pseries,host-serial={passthrough|string},host-model={passthrough|string}’

Only the SE and TM keywords are returned at the moment.
SE for Machine or Cabinet Serial Number and
TM for Machine Type and Model

The SE and TM keywords are controlled by 'host-serial' and 'host-model'
options, respectively. In the case of 'passthrough', the SE shows the
host 'system-id' information and the TM shows the host 'model' information.

Powerpc-utils tools can dispatch RTAS calls to retrieve host
information using this ibm,get-vpd interface. The 'host-serial'
and 'host-model' nodes of device-tree hold the same information but
in a static manner, which is useless after a migration operation.

Signed-off-by: Maxiwell S. Garcia 
---
 hw/ppc/spapr_rtas.c| 110 +
 include/hw/ppc/spapr.h |  12 -
 2 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index 7a2cb786a3..778abcef91 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -287,6 +287,112 @@ static void rtas_ibm_set_system_parameter(PowerPCCPU *cpu,
 rtas_st(rets, 0, ret);
 }
 
+static inline int vpd_st(target_ulong addr, target_ulong len,
+ const void *val, uint16_t val_len)
+{
+hwaddr phys = ppc64_phys_to_real(addr);
+if (len < val_len) {
+return RTAS_OUT_PARAM_ERROR;
+}
+cpu_physical_memory_write(phys, val, val_len);
+return RTAS_OUT_SUCCESS;
+}
+
+static inline void vpd_ret(target_ulong rets, const int status,
+   const int next_seq_number, const int bytes_returned)
+{
+rtas_st(rets, 0, status);
+rtas_st(rets, 1, next_seq_number);
+rtas_st(rets, 2, bytes_returned);
+}
+
+static struct {
+char *keyword;
+char *value;
+} rtas_get_vpd_fields[RTAS_IBM_GET_VPD_KEYWORDS_MAX + 1];
+
+static void rtas_ibm_get_vpd_fields_register(sPAPRMachineState *sm)
+{
+int i = 0;
+char *buf = NULL;
+
+memset(rtas_get_vpd_fields, 0, sizeof(rtas_get_vpd_fields));
+
+buf = spapr_get_valid_host_serial(sm);
+if (buf) {
+rtas_get_vpd_fields[i].keyword = g_strdup("SE");
+rtas_get_vpd_fields[i++].value = g_strdup(buf);
+g_free(buf);
+}
+buf = spapr_get_valid_host_model(sm);
+if (buf) {
+rtas_get_vpd_fields[i].keyword = g_strdup("TM");
+rtas_get_vpd_fields[i++].value = g_strdup(buf);
+g_free(buf);
+}
+}
+
+static void rtas_ibm_get_vpd(PowerPCCPU *cpu,
+ sPAPRMachineState *spapr,
+ uint32_t token, uint32_t nargs,
+ target_ulong args,
+ uint32_t nret, target_ulong rets)
+{
+target_ulong loc_code_addr;
+target_ulong work_area_addr;
+target_ulong work_area_size;
+target_ulong seq_number;
+unsigned char loc_code = 0;
+unsigned int next_seq_number = 1;
+int status = RTAS_IBM_GET_VPD_PARAMETER_ERROR;
+int ret = RTAS_OUT_PARAM_ERROR;
+char *vpd_field = NULL;
+unsigned int vpd_field_len = 0;
+
+/* RTAS not authorized if no keywords have been registered */
+if (!rtas_get_vpd_fields[0].keyword) {
+vpd_ret(rets, RTAS_OUT_NOT_AUTHORIZED, 1, 0);
+return;
+}
+
+loc_code_addr = rtas_ld(args, 0);
+work_area_addr = rtas_ld(args, 1);
+work_area_size = rtas_ld(args, 2);
+seq_number = rtas_ld(args, 3);
+
+/* Specific Location Code is not supported and seq_number */
+/* must be checked to avoid out of bound index error */
+cpu_physical_memory_read(loc_code_addr, _code, 1);
+if ((loc_code != 0) || (seq_number <= 0) ||
+(seq_number > RTAS_IBM_GET_VPD_KEYWORDS_MAX)) {
+vpd_ret(rets, RTAS_IBM_GET_VPD_PARAMETER_ERROR, 1, 0);
+return;
+}
+
+vpd_field = g_strdup_printf("%s %s",
+rtas_get_vpd_fields[seq_number - 1].keyword,
+rtas_get_vpd_fields[seq_number - 1].value);
+
+if (vpd_field) {
+vpd_field_len = strlen(vpd_field);
+ret = vpd_st(work_area_addr, work_area_size,
+ vpd_field, vpd_field_len + 1);
+
+if (ret == 0) {
+next_seq_number = seq_number + 1;
+if (rtas_get_vpd_fields[next_seq_number - 1].keyword) {
+status = RTAS_IBM_GET_VPD_CONTINUE;
+} else {
+status = RTAS_IBM_GET_VPD_SUCCESS;
+next_seq_number = 1;
+}
+}
+}
+
+vpd_ret(rets, status, next_seq_number, vpd_field_len);
+g_free(vpd_field);
+}
+
 static void rtas_ibm_os_term(PowerPCCPU *cpu,
 sPAPRMachineState *spapr,
 uint32_t token, uint32_t nargs,
@@ -464,6 +570,8 @@ void spapr_load_rtas(sPAPRMachineState 

[Qemu-devel] [PATCH v5 0/2] spapr-rtas: add ibm, get-vpd RTAS interface

2019-03-11 Thread Maxiwell S. Garcia
Here are two patches to add a handler for ibm,get-vpd RTAS calls.
This RTAS exposes host information in case of set QEMU options
'host-serial' and 'host-model' as 'passthrough'.

The patch 1 creates helper functions to get valid 'host-serial'
and 'host-model' parameters, guided by QEMU command line. These
parameters are useful to build the guest device tree and to return
get-vpd RTAS calls. The patch 2 adds the ibm,get-vpd itself.


Maxiwell S. Garcia (2):
  spapr: helper functions to get valid host fields
  spapr-rtas: add ibm,get-vpd RTAS interface

 hw/ppc/spapr.c |  58 +-
 hw/ppc/spapr_rtas.c| 110 +
 include/hw/ppc/spapr.h |  15 +-
 3 files changed, 160 insertions(+), 23 deletions(-)

-- 
2.20.1




[Qemu-devel] [PATCH v5 1/2] spapr: helper functions to get valid host fields

2019-03-11 Thread Maxiwell S. Garcia
The pseries options 'host-serial' and 'host-model' accepts
'none', 'passthrough', or  content. The helper
functions in this commit return a valid host field based on
user options.

Signed-off-by: Maxiwell S. Garcia 
---
 hw/ppc/spapr.c | 58 ++
 include/hw/ppc/spapr.h |  3 +++
 2 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9e01226e18..a3078f0261 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1202,6 +1202,34 @@ static void spapr_dt_chosen(sPAPRMachineState *spapr, 
void *fdt)
 g_free(bootlist);
 }
 
+char *spapr_get_valid_host_serial(sPAPRMachineState *spapr)
+{
+char *host_serial = NULL;
+if (spapr->host_serial && !g_str_equal(spapr->host_serial, "none")) {
+if (g_str_equal(spapr->host_serial, "passthrough")) {
+/* -M host-serial=passthrough */
+kvmppc_get_host_serial(_serial);
+} else {
+host_serial = g_strdup(spapr->host_serial);
+}
+}
+return host_serial;
+}
+
+char *spapr_get_valid_host_model(sPAPRMachineState *spapr)
+{
+char *host_model = NULL;
+if (spapr->host_model && !g_str_equal(spapr->host_model, "none")) {
+if (g_str_equal(spapr->host_model, "passthrough")) {
+/* -M host-model=passthrough */
+kvmppc_get_host_model(_model);
+} else {
+host_model = g_strdup(spapr->host_model);
+}
+}
+return host_model;
+}
+
 static void spapr_dt_hypervisor(sPAPRMachineState *spapr, void *fdt)
 {
 /* The /hypervisor node isn't in PAPR - this is a hack to allow PR
@@ -1247,30 +1275,16 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr)
  * Add info to guest to indentify which host is it being run on
  * and what is the uuid of the guest
  */
-if (spapr->host_model && !g_str_equal(spapr->host_model, "none")) {
-if (g_str_equal(spapr->host_model, "passthrough")) {
-/* -M host-model=passthrough */
-if (kvmppc_get_host_model()) {
-_FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
-g_free(buf);
-}
-} else {
-/* -M host-model= */
-_FDT(fdt_setprop_string(fdt, 0, "host-model", spapr->host_model));
-}
+buf = spapr_get_valid_host_model(spapr);
+if (buf) {
+_FDT(fdt_setprop_string(fdt, 0, "host-model", buf));
+g_free(buf);
 }
 
-if (spapr->host_serial && !g_str_equal(spapr->host_serial, "none")) {
-if (g_str_equal(spapr->host_serial, "passthrough")) {
-/* -M host-serial=passthrough */
-if (kvmppc_get_host_serial()) {
-_FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
-g_free(buf);
-}
-} else {
-/* -M host-serial= */
-_FDT(fdt_setprop_string(fdt, 0, "host-serial", 
spapr->host_serial));
-}
+buf = spapr_get_valid_host_serial(spapr);
+if (buf) {
+_FDT(fdt_setprop_string(fdt, 0, "host-serial", buf));
+g_free(buf);
 }
 
 buf = qemu_uuid_unparse_strdup(_uuid);
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 59073a7579..f7ea99dc69 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -842,6 +842,9 @@ int spapr_caps_post_migration(sPAPRMachineState *spapr);
 
 void spapr_check_pagesize(sPAPRMachineState *spapr, hwaddr pagesize,
   Error **errp);
+
+char *spapr_get_valid_host_serial(sPAPRMachineState *spapr);
+char *spapr_get_valid_host_model(sPAPRMachineState *spapr);
 /*
  * XIVE definitions
  */
-- 
2.20.1




[Qemu-devel] [PULL 14/27] pflash: Clean up after commit 368a354f02b, part 1

2019-03-11 Thread Markus Armbruster
QOMification left parameter @qdev unused in pflash_cfi01_register()
and pflash_cfi02_register().  All callers pass NULL.  Remove.

Signed-off-by: Markus Armbruster 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
Tested-by: Philippe Mathieu-Daudé 
Message-Id: <20190308094610.21210-15-arm...@redhat.com>
---
 hw/arm/collie.c  | 4 ++--
 hw/arm/digic_boards.c| 2 +-
 hw/arm/gumstix.c | 4 ++--
 hw/arm/mainstone.c   | 2 +-
 hw/arm/musicpal.c| 4 ++--
 hw/arm/omap_sx1.c| 4 ++--
 hw/arm/versatilepb.c | 2 +-
 hw/arm/xilinx_zynq.c | 2 +-
 hw/arm/z2.c  | 3 +--
 hw/block/pflash_cfi01.c  | 2 +-
 hw/block/pflash_cfi02.c  | 2 +-
 hw/i386/pc_sysfw.c   | 2 +-
 hw/lm32/lm32_boards.c| 4 ++--
 hw/lm32/milkymist.c  | 2 +-
 hw/microblaze/petalogix_ml605_mmu.c  | 3 +--
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 2 +-
 hw/mips/mips_malta.c | 2 +-
 hw/mips/mips_r4k.c   | 2 +-
 hw/ppc/ppc405_boards.c   | 6 +++---
 hw/ppc/sam460ex.c| 2 +-
 hw/ppc/virtex_ml507.c| 2 +-
 hw/sh4/r2d.c | 2 +-
 include/hw/block/flash.h | 4 ++--
 23 files changed, 31 insertions(+), 33 deletions(-)

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 3ca4e078fe..3707b6b598 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -35,12 +35,12 @@ static void collie_init(MachineState *machine)
 s = sa1110_init(sysmem, collie_binfo.ram_size, machine->cpu_type);
 
 dinfo = drive_get(IF_PFLASH, 0, 0);
-pflash_cfi01_register(SA_CS0, NULL, "collie.fl1", 0x0200,
+pflash_cfi01_register(SA_CS0, "collie.fl1", 0x0200,
 dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
 (64 * 1024), 512, 4, 0x00, 0x00, 0x00, 0x00, 0);
 
 dinfo = drive_get(IF_PFLASH, 0, 1);
-pflash_cfi01_register(SA_CS1, NULL, "collie.fl2", 0x0200,
+pflash_cfi01_register(SA_CS1, "collie.fl2", 0x0200,
 dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
 (64 * 1024), 512, 4, 0x00, 0x00, 0x00, 0x00, 0);
 
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index 9f11dcd11f..15a00a1be3 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -129,7 +129,7 @@ static void digic4_add_k8p3215uqb_rom(DigicBoardState *s, 
hwaddr addr,
 #define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024)
 #define FLASH_K8P3215UQB_SECTOR_SIZE (64 * 1024)
 
-pflash_cfi02_register(addr, NULL, "pflash", FLASH_K8P3215UQB_SIZE,
+pflash_cfi02_register(addr, "pflash", FLASH_K8P3215UQB_SIZE,
   NULL, FLASH_K8P3215UQB_SECTOR_SIZE,
   FLASH_K8P3215UQB_SIZE / FLASH_K8P3215UQB_SECTOR_SIZE,
   DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE,
diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
index 56cb763c4e..304dbeab2f 100644
--- a/hw/arm/gumstix.c
+++ b/hw/arm/gumstix.c
@@ -72,7 +72,7 @@ static void connex_init(MachineState *machine)
 #else
 be = 0;
 #endif
-if (!pflash_cfi01_register(0x, NULL, "connext.rom", connex_rom,
+if (!pflash_cfi01_register(0x, "connext.rom", connex_rom,
dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
sector_len, connex_rom / sector_len,
2, 0, 0, 0, 0, be)) {
@@ -109,7 +109,7 @@ static void verdex_init(MachineState *machine)
 #else
 be = 0;
 #endif
-if (!pflash_cfi01_register(0x, NULL, "verdex.rom", verdex_rom,
+if (!pflash_cfi01_register(0x, "verdex.rom", verdex_rom,
dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
sector_len, verdex_rom / sector_len,
2, 0, 0, 0, 0, be)) {
diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index 0beb5c426b..2a1c1072db 100644
--- a/hw/arm/mainstone.c
+++ b/hw/arm/mainstone.c
@@ -148,7 +148,7 @@ static void mainstone_common_init(MemoryRegion 
*address_space_mem,
 exit(1);
 }
 
-if (!pflash_cfi01_register(mainstone_flash_base[i], NULL,
+if (!pflash_cfi01_register(mainstone_flash_base[i],
i ? "mainstone.flash1" : "mainstone.flash0",
MAINSTONE_FLASH,
blk_by_legacy_dinfo(dinfo),
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index de4a12e496..34866b 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1635,14 +1635,14 @@ static void musicpal_init(MachineState *machine)
  * image is smaller than 32 MB.
  */
 #ifdef 

[Qemu-devel] [PULL 15/27] pflash: Clean up after commit 368a354f02b, part 2

2019-03-11 Thread Markus Armbruster
Our pflash devices are simplistically modelled has having
"num-blocks" sectors of equal size "sector-length".  Real hardware
commonly has sectors of different sizes.  How our "sector-length"
property is related to the physical device's multiple sector sizes
is unclear.

Helper functions pflash_cfi01_register() and pflash_cfi02_register()
create a pflash device, set properties including "sector-length" and
"num-blocks", and realize.  They take parameters @size, @sector_len
and @nb_blocs.

QOMification left parameter @size unused.  Obviously, @size should
match @sector_len and @nb_blocs, i.e. size == sector_len * nb_blocs.
All callers satisfy this.

Remove @nb_blocs and compute it from @size and @sector_len.

Signed-off-by: Markus Armbruster 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Alex Bennée 
Message-Id: <20190308094610.21210-16-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/arm/collie.c  | 5 +++--
 hw/arm/digic_boards.c| 1 -
 hw/arm/gumstix.c | 6 ++
 hw/arm/mainstone.c   | 3 +--
 hw/arm/musicpal.c| 4 ++--
 hw/arm/omap_sx1.c| 6 ++
 hw/arm/versatilepb.c | 1 -
 hw/arm/xilinx_zynq.c | 5 ++---
 hw/arm/z2.c  | 3 +--
 hw/block/pflash_cfi01.c  | 5 +++--
 hw/block/pflash_cfi02.c  | 5 +++--
 hw/i386/pc_sysfw.c   | 6 +-
 hw/lm32/lm32_boards.c| 4 ++--
 hw/lm32/milkymist.c  | 3 +--
 hw/microblaze/petalogix_ml605_mmu.c  | 3 +--
 hw/microblaze/petalogix_s3adsp1800_mmu.c | 3 +--
 hw/mips/mips_malta.c | 2 +-
 hw/mips/mips_r4k.c   | 3 +--
 hw/ppc/ppc405_boards.c   | 6 +++---
 hw/ppc/sam460ex.c| 3 +--
 hw/ppc/virtex_ml507.c| 3 +--
 hw/sh4/r2d.c | 3 +--
 include/hw/block/flash.h | 4 ++--
 23 files changed, 35 insertions(+), 52 deletions(-)

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 3707b6b598..d12604c573 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -9,6 +9,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "hw/hw.h"
 #include "hw/sysbus.h"
 #include "hw/boards.h"
@@ -37,12 +38,12 @@ static void collie_init(MachineState *machine)
 dinfo = drive_get(IF_PFLASH, 0, 0);
 pflash_cfi01_register(SA_CS0, "collie.fl1", 0x0200,
 dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-(64 * 1024), 512, 4, 0x00, 0x00, 0x00, 0x00, 0);
+64 * KiB, 4, 0x00, 0x00, 0x00, 0x00, 0);
 
 dinfo = drive_get(IF_PFLASH, 0, 1);
 pflash_cfi01_register(SA_CS1, "collie.fl2", 0x0200,
 dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-(64 * 1024), 512, 4, 0x00, 0x00, 0x00, 0x00, 0);
+64 * KiB, 4, 0x00, 0x00, 0x00, 0x00, 0);
 
 sysbus_create_simple("scoop", 0x4080, NULL);
 
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index 15a00a1be3..304e4d1a29 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -131,7 +131,6 @@ static void digic4_add_k8p3215uqb_rom(DigicBoardState *s, 
hwaddr addr,
 
 pflash_cfi02_register(addr, "pflash", FLASH_K8P3215UQB_SIZE,
   NULL, FLASH_K8P3215UQB_SECTOR_SIZE,
-  FLASH_K8P3215UQB_SIZE / FLASH_K8P3215UQB_SECTOR_SIZE,
   DIGIC4_ROM_MAX_SIZE / FLASH_K8P3215UQB_SIZE,
   4,
   0x00EC, 0x007E, 0x0003, 0x0001,
diff --git a/hw/arm/gumstix.c b/hw/arm/gumstix.c
index 304dbeab2f..79886ce378 100644
--- a/hw/arm/gumstix.c
+++ b/hw/arm/gumstix.c
@@ -74,8 +74,7 @@ static void connex_init(MachineState *machine)
 #endif
 if (!pflash_cfi01_register(0x, "connext.rom", connex_rom,
dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-   sector_len, connex_rom / sector_len,
-   2, 0, 0, 0, 0, be)) {
+   sector_len, 2, 0, 0, 0, 0, be)) {
 error_report("Error registering flash memory");
 exit(1);
 }
@@ -111,8 +110,7 @@ static void verdex_init(MachineState *machine)
 #endif
 if (!pflash_cfi01_register(0x, "verdex.rom", verdex_rom,
dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-   sector_len, verdex_rom / sector_len,
-   2, 0, 0, 0, 0, be)) {
+   sector_len, 2, 0, 0, 0, 0, be)) {
 error_report("Error registering flash memory");
 exit(1);
 }
diff --git a/hw/arm/mainstone.c b/hw/arm/mainstone.c
index 2a1c1072db..e96738ad26 100644
--- 

Re: [Qemu-devel] [PATCH] qom: use object_new_with_type in object_new_with_propv

2019-03-11 Thread Wei Yang
On Mon, Mar 11, 2019 at 06:08:10PM +0100, Marc-André Lureau wrote:
>Hi
>
>On Mon, Mar 11, 2019 at 9:34 AM Wei Yang  wrote:
>>
>> Function object_new_with_propv already get the Type of the object, so we
>> could leverage object_new_with_type here.
>>
>> [make check test pass]
>>
>> Signed-off-by: Wei Yang 
>> ---
>>  qom/object.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qom/object.c b/qom/object.c
>> index 05a8567041..76d2f1eb2f 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -640,7 +640,7 @@ Object *object_new_with_propv(const char *typename,
>>  error_setg(errp, "object type '%s' is abstract", typename);
>>  return NULL;
>>  }
>> -obj = object_new(typename);
>> +obj = object_new_with_type(klass->type);
>>
>>  if (object_set_propv(obj, _err, vargs) < 0) {
>>  goto error;
>> --
>> 2.19.1
>>
>>
>
>Reviewed-by: Marc-André Lureau 
>

Thanks :-)

>-- 
>Marc-André Lureau

-- 
Wei Yang
Help you, Help me



[Qemu-devel] [PULL 26/27] pc: Support firmware configuration with -blockdev

2019-03-11 Thread Markus Armbruster
The PC machines put firmware in ROM by default.  To get it put into
flash memory (required by OVMF), you have to use -drive
if=pflash,unit=0,... and optionally -drive if=pflash,unit=1,...

Why two -drive?  This permits setting up one part of the flash memory
read-only, and the other part read/write.  It also makes upgrading
firmware on the host easier.  Below the hood, it creates two separate
flash devices, because we were too lazy to improve our flash device
models to support sector protection.

The problem at hand is to do the same with -blockdev somehow, as one
more step towards deprecating -drive.

Mapping -drive if=none,... to -blockdev is a solved problem.  With
if=T other than if=none, -drive additionally configures a block device
frontend.  For non-onboard devices, that part maps to -device.  Also a
solved problem.  For onboard devices such as PC flash memory, we have
an unsolved problem.

This is actually an instance of a wider problem: our general device
configuration interface doesn't cover onboard devices.  Instead, we have
a zoo of ad hoc interfaces that are much more limited.  One of them is
-drive, which we'd rather deprecate, but can't until we have suitable
replacements for all its uses.

Sadly, I can't attack the wider problem today.  So back to the narrow
problem.

My first idea was to reduce it to its solved buddy by using pluggable
instead of onboard devices for the flash memory.  Workable, but it
requires some extra smarts in firmware descriptors and libvirt.  Paolo
had an idea that is simpler for libvirt: keep the devices onboard, and
add machine properties for their block backends.

The implementation is less than straightforward, I'm afraid.

First, block backend properties are *qdev* properties.  Machines can't
have those, as they're not devices.  I could duplicate these qdev
properties as QOM properties, but I hate that.

More seriously, the properties do not belong to the machine, they
belong to the onboard flash devices.  Adding them to the machine would
then require bad magic to somehow transfer them to the flash devices.
Fortunately, QOM provides the means to handle exactly this case: add
alias properties to the machine that forward to the onboard devices'
properties.

Properties need to be created in .instance_init() methods.  For PC
machines, that's pc_machine_initfn().  To make alias properties work,
we need to create the onboard flash devices there, too.  Requires
several bug fixes, in the previous commits.  We also have to realize
the devices.  More on that below.

If the user sets pflash0, firmware resides in flash memory.
pc_system_firmware_init() maps and realizes the flash devices.

Else, firmware resides in ROM.  The onboard flash devices aren't used
then.  pc_system_firmware_init() destroys them unrealized, along with
the alias properties.

The existing code to pick up drives defined with -drive if=pflash is
replaced by code to desugar into the machine properties.

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Laszlo Ersek 
Reviewed-by: Michael S. Tsirkin 
Message-Id: <87ftrtux81@dusky.pond.sub.org>
---
 hw/i386/pc.c |   2 +
 hw/i386/pc_sysfw.c   | 233 ---
 include/hw/i386/pc.h |   3 +
 3 files changed, 156 insertions(+), 82 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d8572d3c00..d71dc28ef6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2649,6 +2649,8 @@ static void pc_machine_initfn(Object *obj)
 pcms->smbus_enabled = true;
 pcms->sata_enabled = true;
 pcms->pit_enabled = true;
+
+pc_system_flash_create(pcms);
 }
 
 static void pc_machine_reset(void)
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 785123252c..c628540774 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -40,6 +40,17 @@
 
 #define BIOS_FILENAME "bios.bin"
 
+/*
+ * We don't have a theoretically justifiable exact lower bound on the base
+ * address of any flash mapping. In practice, the IO-APIC MMIO range is
+ * [0xFEE0..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free
+ * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in
+ * size.
+ */
+#define FLASH_SIZE_LIMIT (8 * MiB)
+
+#define FLASH_SECTOR_SIZE 4096
+
 static void pc_isa_bios_init(MemoryRegion *rom_memory,
  MemoryRegion *flash_mem,
  int ram_size)
@@ -71,96 +82,118 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory,
 memory_region_set_readonly(isa_bios, true);
 }
 
-#define FLASH_MAP_UNIT_MAX 2
+static PFlashCFI01 *pc_pflash_create(PCMachineState *pcms,
+ const char *name,
+ const char *alias_prop_name)
+{
+DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01);
 
-/* We don't have a theoretically justifiable exact lower bound on the base
- * address of any flash mapping. In practice, the IO-APIC MMIO range is
- * 

Re: [Qemu-devel] [PATCH v3] exec.c: refactor function flatview_add_to_dispatch()

2019-03-11 Thread Wei Yang
On Mon, Mar 11, 2019 at 02:42:58PM +0100, Paolo Bonzini wrote:
>On 11/03/19 06:42, Wei Yang wrote:
>> flatview_add_to_dispatch() registers page based on the condition of
>> *section*, which may looks like this:
>> 
>> |s|PPP|s|
>> 
>> where s stands for subpage and P for page.
>> 
>> The procedure of this function could be described as:
>> 
>> - register first subpage
>> - register page
>> - register last subpage
>> 
>> This means the procedure could be simplified into these three steps
>> instead of a loop iteration.
>> 
>> This patch refactors the function into three corresponding steps and
>> adds some comment to clarify it.
>> 
>> Signed-off-by: Wei Yang 
>> 
>> ---
>> v3:
>>   * pass int128_make64() instead of 0 to int128_gt()
>> v2:
>>   * removes the loop iteration as suggested by Paolo
>> ---
>>  exec.c | 50 +-
>>  1 file changed, 33 insertions(+), 17 deletions(-)
>> 
>> diff --git a/exec.c b/exec.c
>> index 0959b00c06..79cd561b3b 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -1598,34 +1598,50 @@ static void register_multipage(FlatView *fv,
>>  phys_page_set(d, start_addr >> TARGET_PAGE_BITS, num_pages, 
>> section_index);
>>  }
>>  
>> +/*
>> + * The range in *section* may look like this:
>> + *
>> + *  |s|PPP|s|
>> + *
>> + * where s stands for subpage and P for page.
>> + *
>> + * The procedure in following function could be described as:
>> + *
>> + * - register first subpage
>> + * - register page
>> + * - register last subpage
>
>The last paragraph is a duplicate of the comments in the function, so it
>can be deleted.  I did that and queued the patch.
>

Thanks :-)

>Paolo
>
>> + */
>>  void flatview_add_to_dispatch(FlatView *fv, MemoryRegionSection *section)
>>  {
>> -MemoryRegionSection now = *section, remain = *section;
>> +MemoryRegionSection now, remain = *section;
>>  Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
>>  
>> -if (now.offset_within_address_space & ~TARGET_PAGE_MASK) {
>> -uint64_t left = TARGET_PAGE_ALIGN(now.offset_within_address_space)
>> -   - now.offset_within_address_space;
>> +/* register first subpage */
>> +if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
>> +uint64_t left = 
>> TARGET_PAGE_ALIGN(remain.offset_within_address_space)
>> +- remain.offset_within_address_space;
>>  
>> +now = remain;
>>  now.size = int128_min(int128_make64(left), now.size);
>> +remain.size = int128_sub(remain.size, now.size);
>> +remain.offset_within_address_space += int128_get64(now.size);
>> +remain.offset_within_region += int128_get64(now.size);
>>  register_subpage(fv, );
>> -} else {
>> -now.size = int128_zero();
>>  }
>> -while (int128_ne(remain.size, now.size)) {
>> +
>> +/* register page */
>> +if (int128_ge(remain.size, page_size)) {
>> +now = remain;
>> +now.size = int128_and(now.size, int128_neg(page_size));
>>  remain.size = int128_sub(remain.size, now.size);
>>  remain.offset_within_address_space += int128_get64(now.size);
>>  remain.offset_within_region += int128_get64(now.size);
>> -now = remain;
>> -if (int128_lt(remain.size, page_size)) {
>> -register_subpage(fv, );
>> -} else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
>> -now.size = page_size;
>> -register_subpage(fv, );
>> -} else {
>> -now.size = int128_and(now.size, int128_neg(page_size));
>> -register_multipage(fv, );
>> -}
>> +register_multipage(fv, );
>> +}
>> +
>> +/* register last subpage */
>> +if (int128_gt(remain.size, int128_make64(0))) {
>> +register_subpage(fv, );
>>  }
>>  }
>>  
>> 

-- 
Wei Yang
Help you, Help me



[Qemu-devel] [PULL 12/27] hw/mips/malta: Restrict 'bios_size' variable scope

2019-03-11 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

The 'bios_size' variable is only used in the 'if (!kernel_filename &&
!dinfo)' clause. This is the case when we don't provide -pflash command
line option, and also don't provide a -kernel option. In this case we
will check for the -bios option, or use the default BIOS_FILENAME file.

The 'bios' term is valid in this if statement, but is confuse in the
whole mips_malta_init() scope. Restrict his scope.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Signed-off-by: Markus Armbruster 
Message-Id: <20190308094610.21210-13-arm...@redhat.com>
---
 hw/mips/mips_malta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 51a3ecab59..4dfe06a1a0 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1193,7 +1193,6 @@ void mips_malta_init(MachineState *machine)
 MemoryRegion *ram_low_preio = g_new(MemoryRegion, 1);
 MemoryRegion *ram_low_postio;
 MemoryRegion *bios, *bios_copy = g_new(MemoryRegion, 1);
-target_long bios_size = FLASH_SIZE;
 const size_t smbus_eeprom_size = 8 * 256;
 uint8_t *smbus_eeprom_buf = g_malloc0(smbus_eeprom_size);
 int64_t kernel_entry, bootloader_run_addr;
@@ -1301,6 +1300,7 @@ void mips_malta_init(MachineState *machine)
  bootloader_run_addr, kernel_entry);
 }
 } else {
+target_long bios_size = FLASH_SIZE;
 /* The flash region isn't executable from a KVM guest */
 if (kvm_enabled()) {
 error_report("KVM enabled but no -kernel argument was specified. "
-- 
2.17.2




[Qemu-devel] [PULL 25/27] pc_sysfw: Pass PCMachineState to pc_system_firmware_init()

2019-03-11 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

pc_system_firmware_init() parameter @isapc_ram_fw is PCMachineState
member pci_enabled negated.  The next commit will need more of
PCMachineState.  To prepare for that, pass a PCMachineState *, and
drop the now redundant parameter @isapc_ram_fw.

Signed-off-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Laszlo Ersek 
Message-Id: <20190308131445.17502-11-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 hw/i386/pc.c | 2 +-
 hw/i386/pc_sysfw.c   | 5 -
 include/hw/i386/pc.h | 3 +--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c6d047b42b..d8572d3c00 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1809,7 +1809,7 @@ void pc_memory_init(PCMachineState *pcms,
 }
 
 /* Initialize PC system firmware */
-pc_system_firmware_init(rom_memory, !pcmc->pci_enabled);
+pc_system_firmware_init(pcms, rom_memory);
 
 option_rom_mr = g_malloc(sizeof(*option_rom_mr));
 memory_region_init_ram(option_rom_mr, NULL, "pc.rom", PC_ROM_SIZE,
diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 46b87afe23..785123252c 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -231,8 +231,11 @@ static void old_pc_system_rom_init(MemoryRegion 
*rom_memory, bool isapc_ram_fw)
 bios);
 }
 
-void pc_system_firmware_init(MemoryRegion *rom_memory, bool isapc_ram_fw)
+void pc_system_firmware_init(PCMachineState *pcms,
+ MemoryRegion *rom_memory)
 {
+PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+bool isapc_ram_fw = !pcmc->pci_enabled;
 DriveInfo *pflash_drv;
 
 pflash_drv = drive_get(IF_PFLASH, 0, 0);
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 54222a202d..4f5ed7cefc 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -277,8 +277,7 @@ extern PCIDevice *piix4_dev;
 int piix4_init(PCIBus *bus, ISABus **isa_bus, int devfn);
 
 /* pc_sysfw.c */
-void pc_system_firmware_init(MemoryRegion *rom_memory,
- bool isapc_ram_fw);
+void pc_system_firmware_init(PCMachineState *pcms, MemoryRegion *rom_memory);
 
 /* acpi-build.c */
 void pc_madt_cpu_entry(AcpiDeviceIf *adev, int uid,
-- 
2.17.2




[Qemu-devel] [PULL 13/27] mips_malta: Clean up definition of flash memory size somewhat

2019-03-11 Thread Markus Armbruster
pflash_cfi01_register() takes a size in bytes, a block size in bytes
and a number of blocks.  mips_malta_init() passes BIOS_SIZE, 65536,
FLASH_SIZE >> 16.  Actually consistent only because BIOS_SIZE (defined
in include/hw/mips/bios.h as (4 * MiB)) matches FLASH_SIZE (defined
locally as 0x40).  Confusing all the same.

Pass FLASH_SIZE instead of BIOS_SIZE.

Cc: Aurelien Jarno 
Cc: Aleksandar Rikalo 
Signed-off-by: Markus Armbruster 
Reviewed-by: Alex Bennée 
Reviewed-by: Richard Henderson 
Message-Id: <20190308094610.21210-14-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
---
 hw/mips/mips_malta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 4dfe06a1a0..2f20f56458 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1262,7 +1262,7 @@ void mips_malta_init(MachineState *machine)
 /* Load firmware in flash / BIOS. */
 dinfo = drive_get(IF_PFLASH, 0, fl_idx);
 fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios",
-   BIOS_SIZE,
+   FLASH_SIZE,
dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
65536, FLASH_SIZE >> 16,
4, 0x, 0x, 0x, 0x, be);
-- 
2.17.2




[Qemu-devel] [PULL 23/27] pflash_cfi01: Add pflash_cfi01_get_blk() helper

2019-03-11 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Add an helper to access the opaque struct PFlashCFI01.

Signed-off-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Laszlo Ersek 
Message-Id: <20190308131445.17502-9-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 hw/block/pflash_cfi01.c  | 5 +
 include/hw/block/flash.h | 1 +
 2 files changed, 6 insertions(+)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 9d1c356eb6..125f70b8e4 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -967,6 +967,11 @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base,
 return PFLASH_CFI01(dev);
 }
 
+BlockBackend *pflash_cfi01_get_blk(PFlashCFI01 *fl)
+{
+return fl->blk;
+}
+
 MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl)
 {
 return >mem;
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 914932eaec..a0f488732a 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -22,6 +22,7 @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base,
uint16_t id0, uint16_t id1,
uint16_t id2, uint16_t id3,
int be);
+BlockBackend *pflash_cfi01_get_blk(PFlashCFI01 *fl);
 MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl);
 
 /* pflash_cfi02.c */
-- 
2.17.2




[Qemu-devel] [PULL 27/27] docs/interop/firmware.json: Prefer -machine to if=pflash

2019-03-11 Thread Markus Armbruster
The previous commit added a way to configure firmware with -blockdev
rather than -drive if=pflash.  Document it as the preferred way.

Signed-off-by: Markus Armbruster 
Message-Id: <20190308131445.17502-13-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Laszlo Ersek 
---
 docs/interop/firmware.json | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index 28f9bc1591..ff8c2ce5f2 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -212,9 +212,13 @@
 #
 # @executable: Identifies the firmware executable. The firmware
 #  executable may be shared by multiple virtual machine
-#  definitions. The corresponding QEMU command line option
-#  is "-drive
-#  
if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format".
+#  definitions. The preferred corresponding QEMU command
+#  line options are
+#  -drive 
if=none,id=pflash0,readonly=on,file=@executable.@filename,format=@executable.@format
+#  -machine pflash0=pflash0
+#  or equivalent -blockdev instead of -drive.
+#  With QEMU versions older than 4.0, you have to use
+#  -drive 
if=pflash,unit=0,readonly=on,file=@executable.@filename,format=@executable.@format
 #
 # @nvram-template: Identifies the NVRAM template compatible with
 #  @executable. Management software instantiates an
@@ -225,9 +229,13 @@
 #  individual copies of it are. An NVRAM file is
 #  typically used for persistently storing the
 #  non-volatile UEFI variables of a virtual machine
-#  definition. The corresponding QEMU command line
-#  option is "-drive
-#  
if=pflash,unit=1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format".
+#  definition. The preferred corresponding QEMU
+#  command line options are
+#  -drive 
if=none,id=pflash1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format
+#  -machine pflash1=pflash1
+#  or equivalent -blockdev instead of -drive.
+#  With QEMU versions older than 4.0, you have to use
+#  -drive 
if=pflash,unit=1,readonly=off,file=FILENAME_OF_PRIVATE_NVRAM_FILE,format=@nvram-template.@format
 #
 # Since: 3.0
 ##
-- 
2.17.2




[Qemu-devel] [PULL 21/27] vl: Factor configure_blockdev() out of main()

2019-03-11 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190308131445.17502-7-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 vl.c | 74 ++--
 1 file changed, 42 insertions(+), 32 deletions(-)

diff --git a/vl.c b/vl.c
index ef7f8c36b6..c22ca447fa 100644
--- a/vl.c
+++ b/vl.c
@@ -1199,6 +1199,47 @@ typedef struct BlockdevOptionsQueueEntry {
 
 typedef QSIMPLEQ_HEAD(, BlockdevOptionsQueueEntry) BlockdevOptionsQueue;
 
+static void configure_blockdev(BlockdevOptionsQueue *bdo_queue,
+   MachineClass *machine_class, int snapshot)
+{
+/*
+ * If the currently selected machine wishes to override the
+ * units-per-bus property of its default HBA interface type, do so
+ * now.
+ */
+if (machine_class->units_per_default_bus) {
+override_max_devs(machine_class->block_default_type,
+  machine_class->units_per_default_bus);
+}
+
+/* open the virtual block devices */
+while (!QSIMPLEQ_EMPTY(bdo_queue)) {
+BlockdevOptionsQueueEntry *bdo = QSIMPLEQ_FIRST(bdo_queue);
+
+QSIMPLEQ_REMOVE_HEAD(bdo_queue, entry);
+loc_push_restore(>loc);
+qmp_blockdev_add(bdo->bdo, _fatal);
+loc_pop(>loc);
+qapi_free_BlockdevOptions(bdo->bdo);
+g_free(bdo);
+}
+if (snapshot || replay_mode != REPLAY_MODE_NONE) {
+qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
+  NULL, NULL);
+}
+if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
+  _class->block_default_type, _fatal)) {
+/* We printed help */
+exit(0);
+}
+
+default_drive(default_cdrom, snapshot, machine_class->block_default_type, 
2,
+  CDROM_OPTS);
+default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
+default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
+
+}
+
 static QemuOptsList qemu_smp_opts = {
 .name = "smp-opts",
 .implied_opt_name = "cpus",
@@ -4359,38 +4400,7 @@ int main(int argc, char **argv, char **envp)
 ram_mig_init();
 dirty_bitmap_mig_init();
 
-/* If the currently selected machine wishes to override the units-per-bus
- * property of its default HBA interface type, do so now. */
-if (machine_class->units_per_default_bus) {
-override_max_devs(machine_class->block_default_type,
-  machine_class->units_per_default_bus);
-}
-
-/* open the virtual block devices */
-while (!QSIMPLEQ_EMPTY(_queue)) {
-BlockdevOptionsQueueEntry *bdo = QSIMPLEQ_FIRST(_queue);
-
-QSIMPLEQ_REMOVE_HEAD(_queue, entry);
-loc_push_restore(>loc);
-qmp_blockdev_add(bdo->bdo, _fatal);
-loc_pop(>loc);
-qapi_free_BlockdevOptions(bdo->bdo);
-g_free(bdo);
-}
-if (snapshot || replay_mode != REPLAY_MODE_NONE) {
-qemu_opts_foreach(qemu_find_opts("drive"), drive_enable_snapshot,
-  NULL, NULL);
-}
-if (qemu_opts_foreach(qemu_find_opts("drive"), drive_init_func,
-  _class->block_default_type, _fatal)) {
-/* We printed help */
-exit(0);
-}
-
-default_drive(default_cdrom, snapshot, machine_class->block_default_type, 
2,
-  CDROM_OPTS);
-default_drive(default_floppy, snapshot, IF_FLOPPY, 0, FD_OPTS);
-default_drive(default_sdcard, snapshot, IF_SD, 0, SD_OPTS);
+configure_blockdev(_queue, machine_class, snapshot);
 
 qemu_opts_foreach(qemu_find_opts("mon"),
   mon_init_func, NULL, _fatal);
-- 
2.17.2




[Qemu-devel] [PULL 18/27] vl: Fix latent bug with -global and onboard devices

2019-03-11 Thread Markus Armbruster
main() registers the user's -global only after we create the machine
object, i.e. too late for devices created in the machine's
.instance_init().

Fortunately, we know the bug is only latent: the commit before
previous fixed a bug that would've crashed any attempt to create a
device in an .instance_init().

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
Message-Id: <20190308131445.17502-4-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 vl.c | 19 ++-
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/vl.c b/vl.c
index 5278beaae0..4f84d6568f 100644
--- a/vl.c
+++ b/vl.c
@@ -2937,17 +2937,6 @@ static void user_register_global_props(void)
   global_init_func, NULL, NULL);
 }
 
-/*
- * Note: we should see that these properties are actually having a
- * priority: accel < machine < user. This means e.g. when user
- * specifies something in "-global", it'll always be used with highest
- * priority than either machine/accelerator compat properties.
- */
-static void register_global_properties(MachineState *ms)
-{
-user_register_global_props();
-}
-
 int main(int argc, char **argv, char **envp)
 {
 int i;
@@ -3942,6 +3931,8 @@ int main(int argc, char **argv, char **envp)
  */
 loc_set_none();
 
+user_register_global_props();
+
 replay_configure(icount_opts);
 
 if (incoming && !preconfig_exit_requested) {
@@ -4250,12 +4241,6 @@ int main(int argc, char **argv, char **envp)
  machine_class->name, machine_class->deprecation_reason);
 }
 
-/*
- * Register all the global properties, including accel properties,
- * machine properties, and user-specified ones.
- */
-register_global_properties(current_machine);
-
 /*
  * Migration object can only be created after global properties
  * are applied correctly.
-- 
2.17.2




[Qemu-devel] [PULL 04/27] pflash: Rename *CFI_PFLASH* to *PFLASH_CFI*

2019-03-11 Thread Markus Armbruster
pflash_cfi01.c and pflash_cfi02.c start their identifiers with
pflash_cfi01_ and pflash_cfi02_ respectively, except for
CFI_PFLASH01(), TYPE_CFI_PFLASH01, CFI_PFLASH02(), TYPE_CFI_PFLASH02.
Rename for consistency.

Suggested-by: Philippe Mathieu-Daudé 
Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
Message-Id: <20190308094610.21210-5-arm...@redhat.com>
---
 hw/block/pflash_cfi01.c  | 12 ++--
 hw/block/pflash_cfi02.c  | 14 +++---
 include/hw/block/flash.h |  4 ++--
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index d381f14e3c..f75f0a6998 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -59,8 +59,8 @@ do {\
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
-#define CFI_PFLASH01(obj) \
-OBJECT_CHECK(PFlashCFI01, (obj), TYPE_CFI_PFLASH01)
+#define PFLASH_CFI01(obj) \
+OBJECT_CHECK(PFlashCFI01, (obj), TYPE_PFLASH_CFI01)
 
 #define PFLASH_BE  0
 #define PFLASH_SECURE  1
@@ -698,7 +698,7 @@ static const MemoryRegionOps pflash_cfi01_ops = {
 
 static void pflash_cfi01_realize(DeviceState *dev, Error **errp)
 {
-PFlashCFI01 *pfl = CFI_PFLASH01(dev);
+PFlashCFI01 *pfl = PFLASH_CFI01(dev);
 uint64_t total_len;
 int ret;
 uint64_t blocks_per_device, sector_len_per_device, device_len;
@@ -926,7 +926,7 @@ static void pflash_cfi01_class_init(ObjectClass *klass, 
void *data)
 
 
 static const TypeInfo pflash_cfi01_info = {
-.name   = TYPE_CFI_PFLASH01,
+.name   = TYPE_PFLASH_CFI01,
 .parent = TYPE_SYS_BUS_DEVICE,
 .instance_size  = sizeof(PFlashCFI01),
 .class_init = pflash_cfi01_class_init,
@@ -949,7 +949,7 @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base,
uint16_t id2, uint16_t id3,
int be)
 {
-DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH01);
+DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01);
 
 if (blk) {
 qdev_prop_set_drive(dev, "drive", blk, _abort);
@@ -966,7 +966,7 @@ PFlashCFI01 *pflash_cfi01_register(hwaddr base,
 qdev_init_nofail(dev);
 
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-return CFI_PFLASH01(dev);
+return PFLASH_CFI01(dev);
 }
 
 MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl)
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index df32e20bd7..933de99d6a 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -57,8 +57,8 @@ do {   \
 
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
-#define CFI_PFLASH02(obj) \
-OBJECT_CHECK(PFlashCFI02, (obj), TYPE_CFI_PFLASH02)
+#define PFLASH_CFI02(obj) \
+OBJECT_CHECK(PFlashCFI02, (obj), TYPE_PFLASH_CFI02)
 
 struct PFlashCFI02 {
 /*< private >*/
@@ -534,7 +534,7 @@ static const MemoryRegionOps pflash_cfi02_ops_le = {
 
 static void pflash_cfi02_realize(DeviceState *dev, Error **errp)
 {
-PFlashCFI02 *pfl = CFI_PFLASH02(dev);
+PFlashCFI02 *pfl = PFLASH_CFI02(dev);
 uint32_t chip_len;
 int ret;
 Error *local_err = NULL;
@@ -698,7 +698,7 @@ static Property pflash_cfi02_properties[] = {
 
 static void pflash_cfi02_unrealize(DeviceState *dev, Error **errp)
 {
-PFlashCFI02 *pfl = CFI_PFLASH02(dev);
+PFlashCFI02 *pfl = PFLASH_CFI02(dev);
 timer_del(>timer);
 }
 
@@ -713,7 +713,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, 
void *data)
 }
 
 static const TypeInfo pflash_cfi02_info = {
-.name   = TYPE_CFI_PFLASH02,
+.name   = TYPE_PFLASH_CFI02,
 .parent = TYPE_SYS_BUS_DEVICE,
 .instance_size  = sizeof(PFlashCFI02),
 .class_init = pflash_cfi02_class_init,
@@ -738,7 +738,7 @@ PFlashCFI02 *pflash_cfi02_register(hwaddr base,
uint16_t unlock_addr1,
int be)
 {
-DeviceState *dev = qdev_create(NULL, TYPE_CFI_PFLASH02);
+DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI02);
 
 if (blk) {
 qdev_prop_set_drive(dev, "drive", blk, _abort);
@@ -758,5 +758,5 @@ PFlashCFI02 *pflash_cfi02_register(hwaddr base,
 qdev_init_nofail(dev);
 
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-return CFI_PFLASH02(dev);
+return PFLASH_CFI02(dev);
 }
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 51d8f60c65..333005d9ff 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -7,7 +7,7 @@
 
 /* pflash_cfi01.c */
 
-#define TYPE_CFI_PFLASH01 "cfi.pflash01"
+#define TYPE_PFLASH_CFI01 "cfi.pflash01"
 
 typedef struct PFlashCFI01 PFlashCFI01;
 
@@ -24,7 +24,7 @@ MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl);
 
 /* pflash_cfi02.c */
 
-#define TYPE_CFI_PFLASH02 "cfi.pflash02"
+#define TYPE_PFLASH_CFI02 "cfi.pflash02"
 
 typedef struct 

[Qemu-devel] [PULL 07/27] ppc405_boards: Delete stale, disabled DEBUG_BOARD_INIT code

2019-03-11 Thread Markus Armbruster
The disabled DEBUG_BOARD_INIT code goes back to the initial commit
1a6c0886203, and has since seen only mechanical updates.  It sure
feels like useless clutter now.  Delete it.

Suggested-by: Alex Bennée 
Signed-off-by: Markus Armbruster 
Message-Id: <20190308094610.21210-8-arm...@redhat.com>
Reviewed-by: Alex Bennée 
---
 hw/ppc/ppc405_boards.c | 60 --
 1 file changed, 60 deletions(-)

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index f47b15f10e..bb73d6d848 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -48,8 +48,6 @@
 
 #define USE_FLASH_BIOS
 
-//#define DEBUG_BOARD_INIT
-
 /*/
 /* PPC405EP reference board (IBM) */
 /* Standalone board with:
@@ -171,9 +169,6 @@ static void ref405ep_init(MachineState *machine)
 ram_bases[1] = 0x;
 ram_sizes[1] = 0x;
 ram_size = 128 * MiB;
-#ifdef DEBUG_BOARD_INIT
-printf("%s: register cpu\n", __func__);
-#endif
 env = ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
 , , kernel_filename == NULL ? 0 : 1);
 /* allocate SRAM */
@@ -182,9 +177,6 @@ static void ref405ep_init(MachineState *machine)
_fatal);
 memory_region_add_subregion(sysmem, 0xFFF0, sram);
 /* allocate and load BIOS */
-#ifdef DEBUG_BOARD_INIT
-printf("%s: register BIOS\n", __func__);
-#endif
 fl_idx = 0;
 #ifdef USE_FLASH_BIOS
 dinfo = drive_get(IF_PFLASH, 0, fl_idx);
@@ -193,12 +185,6 @@ static void ref405ep_init(MachineState *machine)
 
 bios_size = blk_getlength(blk);
 fl_sectors = (bios_size + 65535) >> 16;
-#ifdef DEBUG_BOARD_INIT
-printf("Register parallel flash %d size %lx"
-   " at addr %lx '%s' %d\n",
-   fl_idx, bios_size, -bios_size,
-   blk_name(blk), fl_sectors);
-#endif
 pflash_cfi02_register((uint32_t)(-bios_size),
   NULL, "ef405ep.bios", bios_size,
   blk, 65536, fl_sectors, 1,
@@ -208,9 +194,6 @@ static void ref405ep_init(MachineState *machine)
 } else
 #endif
 {
-#ifdef DEBUG_BOARD_INIT
-printf("Load BIOS from file\n");
-#endif
 bios = g_new(MemoryRegion, 1);
 memory_region_init_ram(bios, NULL, "ef405ep.bios", BIOS_SIZE,
_fatal);
@@ -239,21 +222,12 @@ static void ref405ep_init(MachineState *machine)
 memory_region_set_readonly(bios, true);
 }
 /* Register FPGA */
-#ifdef DEBUG_BOARD_INIT
-printf("%s: register FPGA\n", __func__);
-#endif
 ref405ep_fpga_init(sysmem, 0xF030);
 /* Register NVRAM */
-#ifdef DEBUG_BOARD_INIT
-printf("%s: register NVRAM\n", __func__);
-#endif
 m48t59_init(NULL, 0xF000, 0, 8192, 1968, 8);
 /* Load kernel */
 linux_boot = (kernel_filename != NULL);
 if (linux_boot) {
-#ifdef DEBUG_BOARD_INIT
-printf("%s: load kernel\n", __func__);
-#endif
 memset(, 0, sizeof(bd));
 bd.bi_memstart = 0x;
 bd.bi_memsize = ram_size;
@@ -325,10 +299,6 @@ static void ref405ep_init(MachineState *machine)
 initrd_size = 0;
 bdloc = 0;
 }
-#ifdef DEBUG_BOARD_INIT
-printf("bdloc " RAM_ADDR_FMT "\n", bdloc);
-printf("%s: Done\n", __func__);
-#endif
 }
 
 static void ref405ep_class_init(ObjectClass *oc, void *data)
@@ -473,15 +443,9 @@ static void taihu_405ep_init(MachineState *machine)
 memory_region_init_alias(_memories[1], NULL,
  "taihu_405ep.ram-1", ram, ram_bases[1],
  ram_sizes[1]);
-#ifdef DEBUG_BOARD_INIT
-printf("%s: register cpu\n", __func__);
-#endif
 ppc405ep_init(sysmem, ram_memories, ram_bases, ram_sizes,
   , , kernel_filename == NULL ? 0 : 1);
 /* allocate and load BIOS */
-#ifdef DEBUG_BOARD_INIT
-printf("%s: register BIOS\n", __func__);
-#endif
 fl_idx = 0;
 #if defined(USE_FLASH_BIOS)
 dinfo = drive_get(IF_PFLASH, 0, fl_idx);
@@ -492,12 +456,6 @@ static void taihu_405ep_init(MachineState *machine)
 /* XXX: should check that size is 2MB */
 //bios_size = 2 * 1024 * 1024;
 fl_sectors = (bios_size + 65535) >> 16;
-#ifdef DEBUG_BOARD_INIT
-printf("Register parallel flash %d size %lx"
-   " at addr %lx '%s' %d\n",
-   fl_idx, bios_size, -bios_size,
-   blk_name(blk), fl_sectors);
-#endif
 pflash_cfi02_register((uint32_t)(-bios_size),
   NULL, "taihu_405ep.bios", bios_size,
   blk, 65536, fl_sectors, 1,
@@ -507,9 +465,6 @@ static void taihu_405ep_init(MachineState *machine)
 } else
 #endif
 {
-#ifdef DEBUG_BOARD_INIT
-printf("Load BIOS from file\n");
-#endif
 if (bios_name == NULL)
 bios_name = BIOS_FILENAME;
   

[Qemu-devel] [PULL 22/27] vl: Create block backends before setting machine properties

2019-03-11 Thread Markus Armbruster
qemu-system-FOO's main() acts on command line arguments in its own
idiosyncratic order.  There's not much method to its madness.
Whenever we find a case where one kind of command line argument needs
to refer to something created for another kind later, we rejigger the
order.

Block devices get created long after machine properties get processed.
Therefore, block device machine properties can be created, but not
set.  No such properties exist.  But the next commit will create some.
Time to rejigger again: create block devices earlier.

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190308131445.17502-8-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 vl.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/vl.c b/vl.c
index c22ca447fa..e9239d55ad 100644
--- a/vl.c
+++ b/vl.c
@@ -4274,6 +4274,13 @@ int main(int argc, char **argv, char **envp)
 exit(0);
 }
 
+/*
+ * Note: we need to create block backends before
+ * machine_set_property(), so machine properties can refer to
+ * them.
+ */
+configure_blockdev(_queue, machine_class, snapshot);
+
 machine_opts = qemu_get_machine_opts();
 qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
  _fatal);
@@ -4400,8 +4407,6 @@ int main(int argc, char **argv, char **envp)
 ram_mig_init();
 dirty_bitmap_mig_init();
 
-configure_blockdev(_queue, machine_class, snapshot);
-
 qemu_opts_foreach(qemu_find_opts("mon"),
   mon_init_func, NULL, _fatal);
 
-- 
2.17.2




[Qemu-devel] [PULL 05/27] hw: Use PFLASH_CFI0{1, 2} and TYPE_PFLASH_CFI0{1, 2}

2019-03-11 Thread Markus Armbruster
We have two open-coded copies of macro PFLASH_CFI01().  Move the macro
to the header, so we can ditch the copies.  Move PFLASH_CFI02() to the
header for symmetry.

We define macros TYPE_PFLASH_CFI01 and TYPE_PFLASH_CFI02 for type name
strings, then mostly use the strings.  If the macros are worth
defining, they are worth using.  Replace the strings by the macros.

Signed-off-by: Markus Armbruster 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
Message-Id: <20190308094610.21210-6-arm...@redhat.com>
---
 hw/arm/vexpress.c| 4 ++--
 hw/arm/virt.c| 3 ++-
 hw/block/pflash_cfi01.c  | 3 ---
 hw/block/pflash_cfi02.c  | 3 ---
 hw/xtensa/xtfpga.c   | 4 ++--
 include/hw/block/flash.h | 4 
 6 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index ed46d2e730..f07134c424 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -515,7 +515,7 @@ static void vexpress_modify_dtb(const struct arm_boot_info 
*info, void *fdt)
 static PFlashCFI01 *ve_pflash_cfi01_register(hwaddr base, const char *name,
  DriveInfo *di)
 {
-DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01);
 
 if (di) {
 qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(di),
@@ -536,7 +536,7 @@ static PFlashCFI01 *ve_pflash_cfi01_register(hwaddr base, 
const char *name,
 qdev_init_nofail(dev);
 
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-return OBJECT_CHECK(PFlashCFI01, (dev), "cfi.pflash01");
+return PFLASH_CFI01(dev);
 }
 
 static void vexpress_common_init(MachineState *machine)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7f66ddad89..1e8485ff7c 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -35,6 +35,7 @@
 #include "hw/arm/arm.h"
 #include "hw/arm/primecell.h"
 #include "hw/arm/virt.h"
+#include "hw/block/flash.h"
 #include "hw/vfio/vfio-calxeda-xgmac.h"
 #include "hw/vfio/vfio-amd-xgbe.h"
 #include "hw/display/ramfb.h"
@@ -878,7 +879,7 @@ static void create_one_flash(const char *name, hwaddr 
flashbase,
  * parameters as the flash devices on the Versatile Express board.
  */
 DriveInfo *dinfo = drive_get_next(IF_PFLASH);
-DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01);
 SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 const uint64_t sectorlength = 256 * 1024;
 
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index f75f0a6998..1c99aa6e4a 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -59,9 +59,6 @@ do {\
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
-#define PFLASH_CFI01(obj) \
-OBJECT_CHECK(PFlashCFI01, (obj), TYPE_PFLASH_CFI01)
-
 #define PFLASH_BE  0
 #define PFLASH_SECURE  1
 
diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 933de99d6a..915c6171a0 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -57,9 +57,6 @@ do {   \
 
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
-#define PFLASH_CFI02(obj) \
-OBJECT_CHECK(PFlashCFI02, (obj), TYPE_PFLASH_CFI02)
-
 struct PFlashCFI02 {
 /*< private >*/
 SysBusDevice parent_obj;
diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index 3d59a7a356..e05ef75a75 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -167,7 +167,7 @@ static PFlashCFI01 *xtfpga_flash_init(MemoryRegion 
*address_space,
   DriveInfo *dinfo, int be)
 {
 SysBusDevice *s;
-DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+DeviceState *dev = qdev_create(NULL, TYPE_PFLASH_CFI01);
 
 qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo),
 _abort);
@@ -181,7 +181,7 @@ static PFlashCFI01 *xtfpga_flash_init(MemoryRegion 
*address_space,
 s = SYS_BUS_DEVICE(dev);
 memory_region_add_subregion(address_space, board->flash->base,
 sysbus_mmio_get_region(s, 0));
-return OBJECT_CHECK(PFlashCFI01, (dev), "cfi.pflash01");
+return PFLASH_CFI01(dev);
 }
 
 static uint64_t translate_phys_addr(void *opaque, uint64_t addr)
diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
index 333005d9ff..aeea3ca99d 100644
--- a/include/hw/block/flash.h
+++ b/include/hw/block/flash.h
@@ -8,6 +8,8 @@
 /* pflash_cfi01.c */
 
 #define TYPE_PFLASH_CFI01 "cfi.pflash01"
+#define PFLASH_CFI01(obj) \
+OBJECT_CHECK(PFlashCFI01, (obj), TYPE_PFLASH_CFI01)
 
 typedef struct PFlashCFI01 PFlashCFI01;
 
@@ -25,6 +27,8 @@ MemoryRegion *pflash_cfi01_get_memory(PFlashCFI01 *fl);
 /* pflash_cfi02.c */
 
 #define TYPE_PFLASH_CFI02 "cfi.pflash02"
+#define PFLASH_CFI02(obj) \
+OBJECT_CHECK(PFlashCFI02, (obj), TYPE_PFLASH_CFI02)
 
 typedef struct PFlashCFI02 PFlashCFI02;

[Qemu-devel] [PULL 17/27] qom: Move compat_props machinery from qdev to QOM

2019-03-11 Thread Markus Armbruster
See the previous commit for rationale.

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190308131445.17502-3-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 hw/core/qdev.c | 39 ---
 include/hw/qdev-core.h |  4 
 include/qom/object.h   |  3 +++
 qom/object.c   | 39 +++
 4 files changed, 42 insertions(+), 43 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 4f3200d54b..f9b6efe509 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -978,45 +978,6 @@ static void device_initfn(Object *obj)
 QLIST_INIT(>gpios);
 }
 
-/*
- * Global property defaults
- * Slot 0: accelerator's global property defaults
- * Slot 1: machine's global property defaults
- * Each is a GPtrArray of of GlobalProperty.
- * Applied in order, later entries override earlier ones.
- */
-static GPtrArray *object_compat_props[2];
-
-/*
- * Set machine's global property defaults to @compat_props.
- * May be called at most once.
- */
-void object_set_machine_compat_props(GPtrArray *compat_props)
-{
-assert(!object_compat_props[1]);
-object_compat_props[1] = compat_props;
-}
-
-/*
- * Set accelerator's global property defaults to @compat_props.
- * May be called at most once.
- */
-void object_set_accelerator_compat_props(GPtrArray *compat_props)
-{
-assert(!object_compat_props[0]);
-object_compat_props[0] = compat_props;
-}
-
-void object_apply_compat_props(Object *obj)
-{
-int i;
-
-for (i = 0; i < ARRAY_SIZE(object_compat_props); i++) {
-object_apply_global_props(obj, object_compat_props[i],
-  _abort);
-}
-}
-
 static void device_post_init(Object *obj)
 {
 /*
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index aa8a3ea782..33ed3b8dde 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -431,10 +431,6 @@ const char *qdev_fw_name(DeviceState *dev);
 
 Object *qdev_get_machine(void);
 
-void object_set_machine_compat_props(GPtrArray *compat_props);
-void object_set_accelerator_compat_props(GPtrArray *compat_props);
-void object_apply_compat_props(Object *obj);
-
 /* FIXME: make this a link<> */
 void qdev_set_parent_bus(DeviceState *dev, BusState *bus);
 
diff --git a/include/qom/object.h b/include/qom/object.h
index e0262962b5..288cdddf44 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -677,6 +677,9 @@ Object *object_new_with_propv(const char *typename,
 
 void object_apply_global_props(Object *obj, const GPtrArray *props,
Error **errp);
+void object_set_machine_compat_props(GPtrArray *compat_props);
+void object_set_accelerator_compat_props(GPtrArray *compat_props);
+void object_apply_compat_props(Object *obj);
 
 /**
  * object_set_props:
diff --git a/qom/object.c b/qom/object.c
index 05a8567041..e3206d6799 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -408,6 +408,45 @@ void object_apply_global_props(Object *obj, const 
GPtrArray *props, Error **errp
 }
 }
 
+/*
+ * Global property defaults
+ * Slot 0: accelerator's global property defaults
+ * Slot 1: machine's global property defaults
+ * Each is a GPtrArray of of GlobalProperty.
+ * Applied in order, later entries override earlier ones.
+ */
+static GPtrArray *object_compat_props[2];
+
+/*
+ * Set machine's global property defaults to @compat_props.
+ * May be called at most once.
+ */
+void object_set_machine_compat_props(GPtrArray *compat_props)
+{
+assert(!object_compat_props[1]);
+object_compat_props[1] = compat_props;
+}
+
+/*
+ * Set accelerator's global property defaults to @compat_props.
+ * May be called at most once.
+ */
+void object_set_accelerator_compat_props(GPtrArray *compat_props)
+{
+assert(!object_compat_props[0]);
+object_compat_props[0] = compat_props;
+}
+
+void object_apply_compat_props(Object *obj)
+{
+int i;
+
+for (i = 0; i < ARRAY_SIZE(object_compat_props); i++) {
+object_apply_global_props(obj, object_compat_props[i],
+  _abort);
+}
+}
+
 static void object_initialize_with_type(void *data, size_t size, TypeImpl 
*type)
 {
 Object *obj = data;
-- 
2.17.2




[Qemu-devel] [PULL 09/27] r2d: Fix flash memory size, sector size, width, device ID

2019-03-11 Thread Markus Armbruster
pflash_cfi02_register() takes a size in bytes, a block size in bytes
and a number of blocks.  r2d_init() passes FLASH_SIZE, 16 * KiB,
FLASH_SIZE >> 16.  Does not compute: size doesn't match block size *
number of blocks.  The latter happens to win: FLASH_SIZE / 4,
i.e. 8MiB.

The best information we have on the physical hardware lists a Cypress
S29PL127J60TFI130 128MiBit NOR flash addressable in words of 16 bits,
in sectors of 4 and 32 Kibiwords.  We don't model multiple sector
sizes.

Fix the flash size from 8 to 16MiB, and adjust the sector size from 16
to 64KiB.  Fix the width from 4 to 2.  While there, supply the real
device IDs 0x0001, 0x227e, 0x2220, 0x2200 instead of zeros.

Cc: Magnus Damm 
Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Message-Id: <20190308094610.21210-10-arm...@redhat.com>
Tested-by: Philippe Mathieu-Daudé 
---
 hw/sh4/r2d.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c
index 28ed6be05b..e2c46b8f8a 100644
--- a/hw/sh4/r2d.c
+++ b/hw/sh4/r2d.c
@@ -43,7 +43,7 @@
 #include "exec/address-spaces.h"
 
 #define FLASH_BASE 0x
-#define FLASH_SIZE 0x0200
+#define FLASH_SIZE (16 * MiB)
 
 #define SDRAM_BASE 0x0c00 /* Physical location of SDRAM: Area 3 */
 #define SDRAM_SIZE 0x0400
@@ -287,12 +287,20 @@ static void r2d_init(MachineState *machine)
 sysbus_mmio_map(busdev, 1, 0x1400080c);
 mmio_ide_init_drives(dev, dinfo, NULL);
 
-/* onboard flash memory */
+/*
+ * Onboard flash memory
+ * According to the old board user document in Japanese (under
+ * NDA) what is referred to as FROM (Area0) is connected via a
+ * 32-bit bus and CS0 to CN8. The docs mention a Cypress
+ * S29PL127J60TFI130 chipsset.  Per the 'S29PL-J 002-00615
+ * Rev. *E' datasheet, it is a 128Mbit NOR parallel flash
+ * addressable in words of 16bit.
+ */
 dinfo = drive_get(IF_PFLASH, 0, 0);
 pflash_cfi02_register(0x0, NULL, "r2d.flash", FLASH_SIZE,
   dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-  16 * KiB, FLASH_SIZE >> 16,
-  1, 4, 0x, 0x, 0x, 0x,
+  64 * KiB, FLASH_SIZE >> 16,
+  1, 2, 0x0001, 0x227e, 0x2220, 0x2200,
   0x555, 0x2aa, 0);
 
 /* NIC: rtl8139 on-board, and 2 slots. */
-- 
2.17.2




[Qemu-devel] [PULL 24/27] pc_sysfw: Remove unused PcSysFwDevice

2019-03-11 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

This structure is not used since commit 6dd2a5c98a.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Laszlo Ersek 
Signed-off-by: Markus Armbruster 
Message-Id: <20190308131445.17502-10-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 hw/i386/pc_sysfw.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
index 34727c5b1f..46b87afe23 100644
--- a/hw/i386/pc_sysfw.c
+++ b/hw/i386/pc_sysfw.c
@@ -40,11 +40,6 @@
 
 #define BIOS_FILENAME "bios.bin"
 
-typedef struct PcSysFwDevice {
-SysBusDevice busdev;
-uint8_t isapc_ram_fw;
-} PcSysFwDevice;
-
 static void pc_isa_bios_init(MemoryRegion *rom_memory,
  MemoryRegion *flash_mem,
  int ram_size)
-- 
2.17.2




[Qemu-devel] [PULL 11/27] hw/mips/malta: Remove fl_sectors variable

2019-03-11 Thread Markus Armbruster
From: Philippe Mathieu-Daudé 

Variable fl_sectors is used just once.  Since
fl_sectors = bios_size >> 16 and bios_size = FLASH_SIZE there,
we can simply use FLASH_SIZE >> 16, and eliminate variable.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Signed-off-by: Markus Armbruster 
Message-Id: <20190308094610.21210-12-arm...@redhat.com>
---
 hw/mips/mips_malta.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 8736d3a00e..51a3ecab59 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1206,7 +1206,6 @@ void mips_malta_init(MachineState *machine)
 DriveInfo *dinfo;
 DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
 int fl_idx = 0;
-int fl_sectors = bios_size >> 16;
 int be;
 
 DeviceState *dev = qdev_create(NULL, TYPE_MIPS_MALTA);
@@ -1266,7 +1265,7 @@ void mips_malta_init(MachineState *machine)
 fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios",
BIOS_SIZE,
dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-   65536, fl_sectors,
+   65536, FLASH_SIZE >> 16,
4, 0x, 0x, 0x, 0x, be);
 bios = pflash_cfi01_get_memory(fl);
 fl_idx++;
-- 
2.17.2




[Qemu-devel] [PULL 19/27] sysbus: Fix latent bug with onboard devices

2019-03-11 Thread Markus Armbruster
The first call of sysbus_get_default() creates the main system bus and
stores it in QOM as "/machine/unattached/sysbus".  This must not
happen before main() creates "/machine", or else container_get() would
"helpfully" create it as "container" object, and the real creation of
"/machine" would later abort with "attempt to add duplicate property
'machine' to object (type 'container')".  Has been that way ever since
we wired up busses in QOM (commit f968fc6892d, v1.2.0).

I believe the bug is latent.  I got it to bite by trying to
qdev_create() a sysbus device from a machine's .instance_init()
method.

The fix is obvious: store the main system bus in QOM right after
creating "/machine".

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Thomas Huth 
Message-Id: <20190308131445.17502-5-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 hw/core/sysbus.c | 3 ---
 vl.c | 4 
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 9f9edbcab9..307cf90a51 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -357,9 +357,6 @@ static void main_system_bus_create(void)
 qbus_create_inplace(main_system_bus, system_bus_info.instance_size,
 TYPE_SYSTEM_BUS, NULL, "main-system-bus");
 OBJECT(main_system_bus)->free = g_free;
-object_property_add_child(container_get(qdev_get_machine(),
-"/unattached"),
-  "sysbus", OBJECT(main_system_bus), NULL);
 }
 
 BusState *sysbus_get_default(void)
diff --git a/vl.c b/vl.c
index 4f84d6568f..22609af3a4 100644
--- a/vl.c
+++ b/vl.c
@@ -3989,6 +3989,10 @@ int main(int argc, char **argv, char **envp)
 }
 object_property_add_child(object_get_root(), "machine",
   OBJECT(current_machine), _abort);
+object_property_add_child(container_get(OBJECT(current_machine),
+"/unattached"),
+  "sysbus", OBJECT(sysbus_get_default()),
+  NULL);
 
 if (machine_class->minimum_page_bits) {
 if (!set_preferred_target_page_bits(machine_class->minimum_page_bits)) 
{
-- 
2.17.2




[Qemu-devel] [PULL 20/27] vl: Improve legibility of BlockdevOptions queue

2019-03-11 Thread Markus Armbruster
Give the queue head type a name: BlockdevOptionsQueue.

Rename the queue entry type from BlockdevOptions_queue to
BlockdevOptionsQueueEntry.

Signed-off-by: Markus Armbruster 
Message-Id: <20190308131445.17502-6-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
Reviewed-by: Philippe Mathieu-Daudé 
---
 vl.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/vl.c b/vl.c
index 22609af3a4..ef7f8c36b6 100644
--- a/vl.c
+++ b/vl.c
@@ -1191,6 +1191,14 @@ static void default_drive(int enable, int snapshot, 
BlockInterfaceType type,
 
 }
 
+typedef struct BlockdevOptionsQueueEntry {
+BlockdevOptions *bdo;
+Location loc;
+QSIMPLEQ_ENTRY(BlockdevOptionsQueueEntry) entry;
+} BlockdevOptionsQueueEntry;
+
+typedef QSIMPLEQ_HEAD(, BlockdevOptionsQueueEntry) BlockdevOptionsQueue;
+
 static QemuOptsList qemu_smp_opts = {
 .name = "smp-opts",
 .implied_opt_name = "cpus",
@@ -2971,13 +2979,7 @@ int main(int argc, char **argv, char **envp)
 Error *err = NULL;
 bool list_data_dirs = false;
 char *dir, **dirs;
-typedef struct BlockdevOptions_queue {
-BlockdevOptions *bdo;
-Location loc;
-QSIMPLEQ_ENTRY(BlockdevOptions_queue) entry;
-} BlockdevOptions_queue;
-QSIMPLEQ_HEAD(, BlockdevOptions_queue) bdo_queue
-= QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
+BlockdevOptionsQueue bdo_queue = QSIMPLEQ_HEAD_INITIALIZER(bdo_queue);
 
 module_call_init(MODULE_INIT_TRACE);
 
@@ -3101,12 +3103,12 @@ int main(int argc, char **argv, char **envp)
 case QEMU_OPTION_blockdev:
 {
 Visitor *v;
-BlockdevOptions_queue *bdo;
+BlockdevOptionsQueueEntry *bdo;
 
 v = qobject_input_visitor_new_str(optarg, "driver",
   _fatal);
 
-bdo = g_new(BlockdevOptions_queue, 1);
+bdo = g_new(BlockdevOptionsQueueEntry, 1);
 visit_type_BlockdevOptions(v, NULL, >bdo,
_fatal);
 visit_free(v);
@@ -4366,7 +4368,7 @@ int main(int argc, char **argv, char **envp)
 
 /* open the virtual block devices */
 while (!QSIMPLEQ_EMPTY(_queue)) {
-BlockdevOptions_queue *bdo = QSIMPLEQ_FIRST(_queue);
+BlockdevOptionsQueueEntry *bdo = QSIMPLEQ_FIRST(_queue);
 
 QSIMPLEQ_REMOVE_HEAD(_queue, entry);
 loc_push_restore(>loc);
-- 
2.17.2




[Qemu-devel] [PULL 16/27] qdev: Fix latent bug with compat_props and onboard devices

2019-03-11 Thread Markus Armbruster
Compatibility properties started life as a qdev property thing: we
supported them only for qdev properties, and implemented them with the
machinery backing command line option -global.

Recent commit fa0cb34d221 put them to use (tacitly) with memory
backend objects (subtypes of TYPE_MEMORY_BACKEND).  To make that
possible, we first moved the work of applying them from the -global
machinery into TYPE_DEVICE's .instance_post_init() method
device_post_init(), in commits ea9ce8934c5 and b66bbee39f6, then made
it available to TYPE_MEMORY_BACKEND's .instance_post_init() method
host_memory_backend_post_init() as object_apply_compat_props(), in
commit 1c3994f6d2a.

Note the code smell: we now have function name starting with object_
in hw/core/qdev.c.  It has to be there rather than in qom/, because it
calls qdev_get_machine() to find the current accelerator's and
machine's compat_props.

Turns out calling qdev_get_machine() there is problematic.  If we
qdev_create() from a machine's .instance_init() method, we call
device_post_init() and thus qdev_get_machine() before main() can
create "/machine" in QOM.  qdev_get_machine() tries to get it with
container_get(), which "helpfully" creates it as "container" object,
and returns that.  object_apply_compat_props() tries to paper over the
problem by doing nothing when the value of qdev_get_machine() isn't a
TYPE_MACHINE.  But the damage is done already: when main() later
attempts to create the real "/machine", it fails with "attempt to add
duplicate property 'machine' to object (type 'container')", and
aborts.

Since no machine .instance_init() calls qdev_create() so far, the bug
is latent.  But since I want to do that, I get to fix the bug first.

Observe that object_apply_compat_props() doesn't actually need the
MachineState, only its the compat_props member of its MachineClass and
AccelClass.  This permits a simple fix: register MachineClass and
AccelClass compat_props with the object_apply_compat_props() machinery
right after these classes get selected.

This is actually similar to how things worked before commits
ea9ce8934c5 and b66bbee39f6, except we now register much earlier.  The
old code registered them only after the machine's .instance_init()
ran, which would've broken compatibility properties for any devices
created there.

Cc: Marc-André Lureau 
Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
Message-Id: <20190308131445.17502-2-arm...@redhat.com>
Reviewed-by: Michael S. Tsirkin 
---
 accel/accel.c  |  1 +
 hw/core/qdev.c | 48 --
 include/hw/qdev-core.h |  2 ++
 vl.c   |  1 +
 4 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/accel/accel.c b/accel/accel.c
index 0d5b370dfd..8deb475b5d 100644
--- a/accel/accel.c
+++ b/accel/accel.c
@@ -66,6 +66,7 @@ static int accel_init_machine(AccelClass *acc, MachineState 
*ms)
 *(acc->allowed) = false;
 object_unref(OBJECT(accel));
 }
+object_set_accelerator_compat_props(acc->compat_props);
 return ret;
 }
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 512ce7ca7a..4f3200d54b 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -978,25 +978,51 @@ static void device_initfn(Object *obj)
 QLIST_INIT(>gpios);
 }
 
+/*
+ * Global property defaults
+ * Slot 0: accelerator's global property defaults
+ * Slot 1: machine's global property defaults
+ * Each is a GPtrArray of of GlobalProperty.
+ * Applied in order, later entries override earlier ones.
+ */
+static GPtrArray *object_compat_props[2];
+
+/*
+ * Set machine's global property defaults to @compat_props.
+ * May be called at most once.
+ */
+void object_set_machine_compat_props(GPtrArray *compat_props)
+{
+assert(!object_compat_props[1]);
+object_compat_props[1] = compat_props;
+}
+
+/*
+ * Set accelerator's global property defaults to @compat_props.
+ * May be called at most once.
+ */
+void object_set_accelerator_compat_props(GPtrArray *compat_props)
+{
+assert(!object_compat_props[0]);
+object_compat_props[0] = compat_props;
+}
+
 void object_apply_compat_props(Object *obj)
 {
-if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {
-MachineState *m = MACHINE(qdev_get_machine());
-MachineClass *mc = MACHINE_GET_CLASS(m);
+int i;
 
-if (m->accelerator) {
-AccelClass *ac = ACCEL_GET_CLASS(m->accelerator);
-
-if (ac->compat_props) {
-object_apply_global_props(obj, ac->compat_props, _abort);
-}
-}
-object_apply_global_props(obj, mc->compat_props, _abort);
+for (i = 0; i < ARRAY_SIZE(object_compat_props); i++) {
+object_apply_global_props(obj, object_compat_props[i],
+  _abort);
 }
 }
 
 static void device_post_init(Object *obj)
 {
+/*
+ * Note: ordered so that the user's global properties take
+ * precedence.
+ */
 object_apply_compat_props(obj);
   

[Qemu-devel] [PULL 00/27] Pflash and firmware configuration patches for 2019-03-11

2019-03-11 Thread Markus Armbruster
The following changes since commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2019-03-11 18:26:37 +)

are available in the Git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-pflash-2019-03-11

for you to fetch changes up to e33763be7cd3769c9ae48e67d775348863fdabdb:

  docs/interop/firmware.json: Prefer -machine to if=pflash (2019-03-11 22:54:25 
+0100)


Pflash and firmware configuration patches for 2019-03-11

* Fix guest-triggerable exit() in cfi.pflash01 device
* Correct flash memory size with odd image files for machine types
  sam460ex, ref405ep, taihu.
* Correct flash memory size, sector siz, width and device ID for
  machine type r2d.
* Support firmware configuration with -blockdev for machine types
  pc-*.


Markus Armbruster (22):
  pflash: Rename pflash_t to PFlashCFI01, PFlashCFI02
  pflash_cfi01: Do not exit() on guest aborting "write to buffer"
  pflash_cfi01: Log use of flawed "write to buffer"
  pflash: Rename *CFI_PFLASH* to *PFLASH_CFI*
  hw: Use PFLASH_CFI0{1,2} and TYPE_PFLASH_CFI0{1,2}
  sam460ex: Don't size flash memory to match backing image
  ppc405_boards: Delete stale, disabled DEBUG_BOARD_INIT code
  ppc405_boards: Don't size flash memory to match backing image
  r2d: Fix flash memory size, sector size, width, device ID
  mips_malta: Delete disabled, broken DEBUG_BOARD_INIT code
  mips_malta: Clean up definition of flash memory size somewhat
  pflash: Clean up after commit 368a354f02b, part 1
  pflash: Clean up after commit 368a354f02b, part 2
  qdev: Fix latent bug with compat_props and onboard devices
  qom: Move compat_props machinery from qdev to QOM
  vl: Fix latent bug with -global and onboard devices
  sysbus: Fix latent bug with onboard devices
  vl: Improve legibility of BlockdevOptions queue
  vl: Factor configure_blockdev() out of main()
  vl: Create block backends before setting machine properties
  pc: Support firmware configuration with -blockdev
  docs/interop/firmware.json: Prefer -machine to if=pflash

Philippe Mathieu-Daudé (5):
  hw/mips/malta: Remove fl_sectors variable
  hw/mips/malta: Restrict 'bios_size' variable scope
  pflash_cfi01: Add pflash_cfi01_get_blk() helper
  pc_sysfw: Remove unused PcSysFwDevice
  pc_sysfw: Pass PCMachineState to pc_system_firmware_init()

 accel/accel.c|   1 +
 docs/interop/firmware.json   |  20 ++-
 hw/arm/collie.c  |   9 +-
 hw/arm/digic_boards.c|   3 +-
 hw/arm/gumstix.c |  10 +-
 hw/arm/mainstone.c   |   5 +-
 hw/arm/musicpal.c|   8 +-
 hw/arm/omap_sx1.c|  10 +-
 hw/arm/versatilepb.c |   3 +-
 hw/arm/vexpress.c|  10 +-
 hw/arm/virt.c|   3 +-
 hw/arm/xilinx_zynq.c |   7 +-
 hw/arm/z2.c  |   6 +-
 hw/block/pflash_cfi01.c  | 128 +---
 hw/block/pflash_cfi02.c  |  81 ++-
 hw/core/qdev.c   |  21 +--
 hw/core/sysbus.c |   3 -
 hw/i386/pc.c |   4 +-
 hw/i386/pc_sysfw.c   | 243 +++
 hw/lm32/lm32_boards.c|   8 +-
 hw/lm32/milkymist.c  |   5 +-
 hw/microblaze/petalogix_ml605_mmu.c  |   6 +-
 hw/microblaze/petalogix_s3adsp1800_mmu.c |   5 +-
 hw/mips/mips_malta.c |  21 +--
 hw/mips/mips_r4k.c   |   5 +-
 hw/ppc/ppc405_boards.c   | 102 ++---
 hw/ppc/sam460ex.c|  42 --
 hw/ppc/virtex_ml507.c|   5 +-
 hw/sh4/r2d.c |  17 ++-
 hw/xtensa/xtfpga.c   |  12 +-
 include/hw/block/flash.h |  58 +---
 include/hw/i386/pc.h |   6 +-
 include/hw/qdev-core.h   |   2 -
 include/qom/object.h |   3 +
 qom/object.c |  39 +
 vl.c | 125 
 36 files changed, 547 insertions(+), 489 deletions(-)

-- 
2.17.2




[Qemu-devel] [PULL 01/27] pflash: Rename pflash_t to PFlashCFI01, PFlashCFI02

2019-03-11 Thread Markus Armbruster
flash.h's incomplete struct pflash_t is completed both in
pflash_cfi01.c and in pflash_cfi02.c.  The complete types are
incompatible.  This can hide type errors, such as passing a pflash_t
created with pflash_cfi02_register() to pflash_cfi01_get_memory().

Furthermore, POSIX reserves typedef names ending with _t.

Rename the two structs to PFlashCFI01 and PFlashCFI02.

Signed-off-by: Markus Armbruster 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Message-Id: <20190308094610.21210-2-arm...@redhat.com>
---
 hw/arm/vexpress.c|  8 ++--
 hw/block/pflash_cfi01.c  | 89 +---
 hw/block/pflash_cfi02.c  | 73 
 hw/i386/pc_sysfw.c   |  2 +-
 hw/mips/mips_malta.c |  2 +-
 hw/xtensa/xtfpga.c   | 10 ++---
 include/hw/block/flash.h | 53 ++--
 7 files changed, 126 insertions(+), 111 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index c02d18ee61..ed46d2e730 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -512,8 +512,8 @@ static void vexpress_modify_dtb(const struct arm_boot_info 
*info, void *fdt)
 /* Open code a private version of pflash registration since we
  * need to set non-default device width for VExpress platform.
  */
-static pflash_t *ve_pflash_cfi01_register(hwaddr base, const char *name,
-  DriveInfo *di)
+static PFlashCFI01 *ve_pflash_cfi01_register(hwaddr base, const char *name,
+ DriveInfo *di)
 {
 DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
 
@@ -536,7 +536,7 @@ static pflash_t *ve_pflash_cfi01_register(hwaddr base, 
const char *name,
 qdev_init_nofail(dev);
 
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
-return OBJECT_CHECK(pflash_t, (dev), "cfi.pflash01");
+return OBJECT_CHECK(PFlashCFI01, (dev), "cfi.pflash01");
 }
 
 static void vexpress_common_init(MachineState *machine)
@@ -548,7 +548,7 @@ static void vexpress_common_init(MachineState *machine)
 qemu_irq pic[64];
 uint32_t sys_id;
 DriveInfo *dinfo;
-pflash_t *pflash0;
+PFlashCFI01 *pflash0;
 I2CBus *i2c;
 ram_addr_t vram_size, sram_size;
 MemoryRegion *sysmem = get_system_memory();
diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index bffb4c40e7..a51ac9f399 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -65,12 +65,13 @@ do {
\
 #define DPRINTF(fmt, ...) do { } while (0)
 #endif
 
-#define CFI_PFLASH01(obj) OBJECT_CHECK(pflash_t, (obj), TYPE_CFI_PFLASH01)
+#define CFI_PFLASH01(obj) \
+OBJECT_CHECK(PFlashCFI01, (obj), TYPE_CFI_PFLASH01)
 
 #define PFLASH_BE  0
 #define PFLASH_SECURE  1
 
-struct pflash_t {
+struct PFlashCFI01 {
 /*< private >*/
 SysBusDevice parent_obj;
 /*< public >*/
@@ -109,17 +110,17 @@ static const VMStateDescription vmstate_pflash = {
 .minimum_version_id = 1,
 .post_load = pflash_post_load,
 .fields = (VMStateField[]) {
-VMSTATE_UINT8(wcycle, pflash_t),
-VMSTATE_UINT8(cmd, pflash_t),
-VMSTATE_UINT8(status, pflash_t),
-VMSTATE_UINT64(counter, pflash_t),
+VMSTATE_UINT8(wcycle, PFlashCFI01),
+VMSTATE_UINT8(cmd, PFlashCFI01),
+VMSTATE_UINT8(status, PFlashCFI01),
+VMSTATE_UINT64(counter, PFlashCFI01),
 VMSTATE_END_OF_LIST()
 }
 };
 
 static void pflash_timer (void *opaque)
 {
-pflash_t *pfl = opaque;
+PFlashCFI01 *pfl = opaque;
 
 trace_pflash_timer_expired(pfl->cmd);
 /* Reset flash */
@@ -133,7 +134,7 @@ static void pflash_timer (void *opaque)
  * If this code is called we know we have a device_width set for
  * this flash.
  */
-static uint32_t pflash_cfi_query(pflash_t *pfl, hwaddr offset)
+static uint32_t pflash_cfi_query(PFlashCFI01 *pfl, hwaddr offset)
 {
 int i;
 uint32_t resp = 0;
@@ -193,7 +194,7 @@ static uint32_t pflash_cfi_query(pflash_t *pfl, hwaddr 
offset)
 
 
 /* Perform a device id query based on the bank width of the flash. */
-static uint32_t pflash_devid_query(pflash_t *pfl, hwaddr offset)
+static uint32_t pflash_devid_query(PFlashCFI01 *pfl, hwaddr offset)
 {
 int i;
 uint32_t resp;
@@ -241,7 +242,7 @@ static uint32_t pflash_devid_query(pflash_t *pfl, hwaddr 
offset)
 return resp;
 }
 
-static uint32_t pflash_data_read(pflash_t *pfl, hwaddr offset,
+static uint32_t pflash_data_read(PFlashCFI01 *pfl, hwaddr offset,
  int width, int be)
 {
 uint8_t *p;
@@ -284,8 +285,8 @@ static uint32_t pflash_data_read(pflash_t *pfl, hwaddr 
offset,
 return ret;
 }
 
-static uint32_t pflash_read (pflash_t *pfl, hwaddr offset,
- int width, int be)
+static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
+int width, int be)
 {
 hwaddr boff;
 uint32_t 

[Qemu-devel] [PULL 06/27] sam460ex: Don't size flash memory to match backing image

2019-03-11 Thread Markus Armbruster
Machine "sam460ex" maps its flash memory at address 0xFFF0.  When
no image is supplied, its size is 1MiB (0x10), and 512KiB of ROM
get mapped on top of its second half.  Else, it's the size of the
image rounded up to the next multiple of 64KiB.

The rounding is actually useless: pflash_cfi01_realize() fails with
"failed to read the initial flash content" unless it's a no-op.

I have no idea what happens when the pflash's size exceeds 1MiB.
Useful outcomes seem unlikely.

I guess memory at the end of the address space remains unmapped when
it's smaller than 1MiB.  Again, useful outcomes seem unlikely.

The physical hardware appears to have 512KiB of flash memory:
https://eu.mouser.com/datasheet/2/268/atmel_AT49BV040B-1180330.pdf

For now, just set the flash memory size to 1MiB regardless of image
size, and document the mess.

Cc: BALATON Zoltan 
Signed-off-by: Markus Armbruster 
Reviewed-by: BALATON Zoltan 
Reviewed-by: Alex Bennée 
Message-Id: <20190308094610.21210-7-arm...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
---
 hw/ppc/sam460ex.c | 41 ++---
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index d455c4bd07..c6258ca1d0 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -91,32 +91,43 @@ struct boot_info {
 
 static int sam460ex_load_uboot(void)
 {
+/*
+ * This first creates 1MiB of flash memory mapped at the end of
+ * the 32-bit address space (0xFFF0..0x).
+ *
+ * If_PFLASH unit 0 is defined, the flash memory is initialized
+ * from that block backend.
+ *
+ * Else, it's initialized to zero.  And then 512KiB of ROM get
+ * mapped on top of its second half (0xFFF8..0x),
+ * initialized from u-boot-sam460-20100605.bin.
+ *
+ * This doesn't smell right.
+ *
+ * The physical hardware appears to have 512KiB flash memory.
+ *
+ * TODO Figure out what we really need here, and clean this up.
+ */
+
 DriveInfo *dinfo;
-BlockBackend *blk = NULL;
-hwaddr base = FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32);
-long bios_size = FLASH_SIZE;
-int fl_sectors;
 
 dinfo = drive_get(IF_PFLASH, 0, 0);
-if (dinfo) {
-blk = blk_by_legacy_dinfo(dinfo);
-bios_size = blk_getlength(blk);
-}
-fl_sectors = (bios_size + 65535) >> 16;
-
-if (!pflash_cfi01_register(base, NULL, "sam460ex.flash", bios_size,
-   blk, 64 * KiB, fl_sectors,
+if (!pflash_cfi01_register(FLASH_BASE | ((hwaddr)FLASH_BASE_H << 32),
+   NULL, "sam460ex.flash", FLASH_SIZE,
+   dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+   64 * KiB, FLASH_SIZE / (64 * KiB),
1, 0x89, 0x18, 0x, 0x0, 1)) {
 error_report("Error registering flash memory");
 /* XXX: return an error instead? */
 exit(1);
 }
 
-if (!blk) {
+if (!dinfo) {
 /*error_report("No flash image given with the 'pflash' parameter,"
 " using default u-boot image");*/
-base = UBOOT_LOAD_BASE | ((hwaddr)FLASH_BASE_H << 32);
-rom_add_file_fixed(UBOOT_FILENAME, base, -1);
+rom_add_file_fixed(UBOOT_FILENAME,
+   UBOOT_LOAD_BASE | ((hwaddr)FLASH_BASE_H << 32),
+   -1);
 }
 
 return 0;
-- 
2.17.2




[Qemu-devel] [PULL 10/27] mips_malta: Delete disabled, broken DEBUG_BOARD_INIT code

2019-03-11 Thread Markus Armbruster
The debug code under DEBUG_BOARD_INIT doesn't compile:

  hw/mips/mips_malta.c:1273:16: error: implicit declaration of function 
‘blk_name’; did you mean ‘basename’? [-Werror=implicit-function-declaration]
blk_name(dinfo->bdrv), fl_sectors);
^~~~
  hw/mips/mips_malta.c:1273:16: error: nested extern declaration of 
‘blk_name’ [-Werror=nested-externs]
  hw/mips/mips_malta.c:1273:30: error: ‘DriveInfo’ {aka ‘struct DriveInfo’} 
has no member named ‘bdrv’
blk_name(dinfo->bdrv), fl_sectors);
^~

Delete it.

Reported-by: Philippe Mathieu-Daudé 
Signed-off-by: Markus Armbruster 
Reviewed-by: Aleksandar Markovic 
Message-Id: <20190308094610.21210-11-arm...@redhat.com>
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
---
 hw/mips/mips_malta.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 2827074e9b..8736d3a00e 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -58,8 +58,6 @@
 #include "exec/semihost.h"
 #include "hw/mips/cps.h"
 
-//#define DEBUG_BOARD_INIT
-
 #define ENVP_ADDR  0x80002000l
 #define ENVP_NB_ENTRIES16
 #define ENVP_ENTRY_SIZE256
@@ -1265,14 +1263,6 @@ void mips_malta_init(MachineState *machine)
 
 /* Load firmware in flash / BIOS. */
 dinfo = drive_get(IF_PFLASH, 0, fl_idx);
-#ifdef DEBUG_BOARD_INIT
-if (dinfo) {
-printf("Register parallel flash %d size " TARGET_FMT_lx " at "
-   "addr %08llx '%s' %x\n",
-   fl_idx, bios_size, FLASH_ADDRESS,
-   blk_name(dinfo->bdrv), fl_sectors);
-}
-#endif
 fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios",
BIOS_SIZE,
dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
-- 
2.17.2




[Qemu-devel] [PULL 08/27] ppc405_boards: Don't size flash memory to match backing image

2019-03-11 Thread Markus Armbruster
Machine "ref405ep" maps its flash memory at address 2^32 - image size.
Image size is rounded up to the next multiple of 64KiB.  Useless,
because pflash_cfi02_realize() fails with "failed to read the initial
flash content" unless the rounding is a no-op.

If the image size exceeds 0x8 Bytes, we overlap first SRAM, then
other stuff.  No idea how that would play out, but useful outcomes
seem unlikely.

Map the flash memory at fixed address 0xFFF8 with size 512KiB,
regardless of image size, to match the physical hardware.

Machine "taihu" maps its boot flash memory similarly.  The code even
has a comment /* XXX: should check that size is 2MB */, followed by
disabled code to adjust the size to 2MiB regardless of image size.

Its code to map its application flash memory looks the same, except
there the XXX comment asks for 32MiB, and the code to adjust the size
isn't disabled.  Note that pflash_cfi02_realize() fails with "failed
to read the initial flash content" for images smaller than 32MiB.

Map the boot flash memory at fixed address 0xFFE0 with size 2MiB,
to match the physical hardware.  Delete dead code from application
flash mapping, and simplify some.

Cc: David Gibson 
Signed-off-by: Markus Armbruster 
Acked-by: David Gibson 
Reviewed-by: Alex Bennée 
Message-Id: <20190308094610.21210-9-arm...@redhat.com>
---
 hw/ppc/ppc405_boards.c | 36 
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index bb73d6d848..fe8e3cad12 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -156,7 +156,7 @@ static void ref405ep_init(MachineState *machine)
 target_ulong kernel_base, initrd_base;
 long kernel_size, initrd_size;
 int linux_boot;
-int fl_idx, fl_sectors, len;
+int len;
 DriveInfo *dinfo;
 MemoryRegion *sysmem = get_system_memory();
 
@@ -177,20 +177,16 @@ static void ref405ep_init(MachineState *machine)
_fatal);
 memory_region_add_subregion(sysmem, 0xFFF0, sram);
 /* allocate and load BIOS */
-fl_idx = 0;
 #ifdef USE_FLASH_BIOS
-dinfo = drive_get(IF_PFLASH, 0, fl_idx);
+dinfo = drive_get(IF_PFLASH, 0, 0);
 if (dinfo) {
-BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
-
-bios_size = blk_getlength(blk);
-fl_sectors = (bios_size + 65535) >> 16;
+bios_size = 8 * MiB;
 pflash_cfi02_register((uint32_t)(-bios_size),
   NULL, "ef405ep.bios", bios_size,
-  blk, 65536, fl_sectors, 1,
+  dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+  64 * KiB, bios_size / (64 * KiB), 1,
   2, 0x0001, 0x22DA, 0x, 0x, 0x555, 0x2AA,
   1);
-fl_idx++;
 } else
 #endif
 {
@@ -425,7 +421,7 @@ static void taihu_405ep_init(MachineState *machine)
 target_ulong kernel_base, initrd_base;
 long kernel_size, initrd_size;
 int linux_boot;
-int fl_idx, fl_sectors;
+int fl_idx;
 DriveInfo *dinfo;
 
 /* RAM is soldered to the board so the size cannot be changed */
@@ -450,15 +446,11 @@ static void taihu_405ep_init(MachineState *machine)
 #if defined(USE_FLASH_BIOS)
 dinfo = drive_get(IF_PFLASH, 0, fl_idx);
 if (dinfo) {
-BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
-
-bios_size = blk_getlength(blk);
-/* XXX: should check that size is 2MB */
-//bios_size = 2 * 1024 * 1024;
-fl_sectors = (bios_size + 65535) >> 16;
-pflash_cfi02_register((uint32_t)(-bios_size),
+bios_size = 2 * MiB;
+pflash_cfi02_register(0xFFE0,
   NULL, "taihu_405ep.bios", bios_size,
-  blk, 65536, fl_sectors, 1,
+  dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+  64 * KiB, bios_size / (64 * KiB), 1,
   4, 0x0001, 0x22DA, 0x, 0x, 0x555, 0x2AA,
   1);
 fl_idx++;
@@ -491,14 +483,10 @@ static void taihu_405ep_init(MachineState *machine)
 /* Register Linux flash */
 dinfo = drive_get(IF_PFLASH, 0, fl_idx);
 if (dinfo) {
-BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
-
-bios_size = blk_getlength(blk);
-/* XXX: should check that size is 32MB */
 bios_size = 32 * MiB;
-fl_sectors = (bios_size + 65535) >> 16;
 pflash_cfi02_register(0xfc00, NULL, "taihu_405ep.flash", bios_size,
-  blk, 65536, fl_sectors, 1,
+  dinfo ? blk_by_legacy_dinfo(dinfo) : NULL,
+  64 * KiB, bios_size / (64 * KiB), 1,
   4, 0x0001, 0x22DA, 0x, 0x, 0x555, 0x2AA,
   1);
 fl_idx++;
-- 

[Qemu-devel] [PULL 02/27] pflash_cfi01: Do not exit() on guest aborting "write to buffer"

2019-03-11 Thread Markus Armbruster
When a guest tries to abort "write to buffer" (command 0xE8), we print
"PFLASH: Possible BUG - Write block confirm", then exit(1).  Letting
the guest terminate QEMU is not a good idea.  Instead, LOG_UNIMP we
screwed up, then reset the device.

Macro PFLASH_BUG() is now unused; delete it.

Suggested-by: Philippe Mathieu-Daudé 
Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
Message-Id: <20190308094610.21210-3-arm...@redhat.com>
---
 hw/block/pflash_cfi01.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index a51ac9f399..e6d933a06d 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -49,12 +49,6 @@
 #include "sysemu/sysemu.h"
 #include "trace.h"
 
-#define PFLASH_BUG(fmt, ...) \
-do { \
-fprintf(stderr, "PFLASH: Possible BUG - " fmt, ## __VA_ARGS__); \
-exit(1); \
-} while(0)
-
 /* #define PFLASH_DEBUG */
 #ifdef PFLASH_DEBUG
 #define DPRINTF(fmt, ...)   \
@@ -623,8 +617,11 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 pfl->wcycle = 0;
 pfl->status |= 0x80;
 } else {
-DPRINTF("%s: unknown command for \"write block\"\n", __func__);
-PFLASH_BUG("Write block confirm");
+qemu_log_mask(LOG_UNIMP,
+"%s: Aborting write to buffer not implemented,"
+" the data is already written to storage!\n"
+"Flash device reset into READ mode.\n",
+__func__);
 goto reset_flash;
 }
 break;
-- 
2.17.2




[Qemu-devel] [PULL 03/27] pflash_cfi01: Log use of flawed "write to buffer"

2019-03-11 Thread Markus Armbruster
Our implementation of "write to buffer" (command 0xE8) is flawed.
LOG_UNIMP its use, and add some FIXME comments.

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
Message-Id: <20190308094610.21210-4-arm...@redhat.com>
---
 hw/block/pflash_cfi01.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index e6d933a06d..d381f14e3c 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -502,6 +502,10 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 break;
 case 0xe8: /* Write to buffer */
 DPRINTF("%s: Write to buffer\n", __func__);
+/* FIXME should save @offset, @width for case 1+ */
+qemu_log_mask(LOG_UNIMP,
+  "%s: Write to buffer emulation is flawed\n",
+  __func__);
 pfl->status |= 0x80; /* Ready! */
 break;
 case 0xf0: /* Probe for AMD flash */
@@ -545,6 +549,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 /* Mask writeblock size based on device width, or bank width if
  * device width not specified.
  */
+/* FIXME check @offset, @width */
 if (pfl->device_width) {
 value = extract32(value, 0, pfl->device_width * 8);
 } else {
@@ -582,7 +587,13 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 case 2:
 switch (pfl->cmd) {
 case 0xe8: /* Block write */
+/* FIXME check @offset, @width */
 if (!pfl->ro) {
+/*
+ * FIXME writing straight to memory is *wrong*.  We
+ * should write to a buffer, and flush it to memory
+ * only on confirm command (see below).
+ */
 pflash_data_write(pfl, offset, value, width, be);
 } else {
 pfl->status |= 0x10; /* Programming error */
@@ -598,6 +609,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 pfl->wcycle++;
 if (!pfl->ro) {
 /* Flush the entire write buffer onto backing storage.  */
+/* FIXME premature! */
 pflash_update(pfl, offset & mask, pfl->writeblock_size);
 } else {
 pfl->status |= 0x10; /* Programming error */
@@ -614,6 +626,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
 switch (pfl->cmd) {
 case 0xe8: /* Block write */
 if (cmd == 0xd0) {
+/* FIXME this is where we should write out the buffer */
 pfl->wcycle = 0;
 pfl->status |= 0x80;
 } else {
-- 
2.17.2




Re: [Qemu-devel] [PULL 4/5] hw/display: Add basic ATI VGA emulation

2019-03-11 Thread BALATON Zoltan

On Mon, 11 Mar 2019, Markus Armbruster wrote:

Mark Cave-Ayland  writes:

On 11/03/2019 19:51, BALATON Zoltan wrote:

On Mon, 11 Mar 2019, Markus Armbruster wrote:

Gerd Hoffmann  writes:

From: BALATON Zoltan 

At least two machines, the PPC mac99 and MIPS fulong2e, have an ATI
gfx chip by default (Rage 128 Pro and M6/RV100 respectively) and
guests running on these and the PMON2000 firmware of the fulong2e
expect this to be available. Fortunately these are very similar chips
so they can be mostly emulated in the same device model. This patch
adds basic emulation of these ATI VGA chips.

While this is incomplete and currently only enough to run the MIPS
firmware and get framebuffer output with Linux, it allows the fulong2e
board to work more like the real hardware and having it in QEMU in
this state provides a way to experiment with it and allows others to
contribute to improve it. It is compiled for all archs but only the
fulong2e (which currently has no display output at all) is set to use
it by default (in a separate patch).

Signed-off-by: BALATON Zoltan 
Acked-by: Aleksandar Markovic 
Tested-by: Andrew Randrianasulu 
Tested-by: Howard Spoelstra 
Message-id:
0b1b7c22873a6e37627261b04fb687412b25ff4f.1552152100.git.bala...@eik.bme.hu
Signed-off-by: Gerd Hoffmann 

[...]

diff --git a/hw/display/trace-events b/hw/display/trace-events
index 37d3264bb2e6..80993cc4d913 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -138,3 +138,7 @@ vga_cirrus_write_blt(uint32_t offset, uint32_t val) "offset
0x%x, val 0x%x"
?sii9022_read_reg(uint8_t addr, uint8_t val) "addr 0x%02x, val 0x%02x"
?sii9022_write_reg(uint8_t addr, uint8_t val) "addr 0x%02x, val 0x%02x"
?sii9022_switch_mode(const char *mode) "mode: %s"
+
+# hw/display/ati*.c
+ati_mm_read(unsigned int size, uint64_t addr, const char *name, uint64_t val) 
"%u
0x%"HWADDR_PRIx " %s -> 0x%"PRIx64
+ati_mm_write(unsigned int size, uint64_t addr, const char *name, uint64_t val)
"%u 0x%"HWADDR_PRIx " %s <- 0x%"PRIx64


Blows up for me:

Traceback (most recent call last):
?File "/work/armbru/qemu/scripts/tracetool.py", line 152, in 
?? main(sys.argv)
?File "/work/armbru/qemu/scripts/tracetool.py", line 147, in main
?? binary=binary, probe_prefix=probe_prefix)
?File "/work/armbru/qemu/scripts/tracetool/__init__.py", line 472, in generate
?? tracetool.format.generate(events, format, backend, group)
?File "/work/armbru/qemu/scripts/tracetool/format/__init__.py", line 85, in 
generate
?? func(events, backend, group)
?File "/work/armbru/qemu/scripts/tracetool/format/log_stap.py", line 121, in 
generate
?? fmt_str = "%d@%d " + e.name + " " + c_fmt_to_stap(e.fmt) + "\\n"
?File "/work/armbru/qemu/scripts/tracetool/format/log_stap.py", line 64, in
c_fmt_to_stap
?? bits.append(c_macro_to_format(macro))
?File "/work/armbru/qemu/scripts/tracetool/format/log_stap.py", line 36, in
c_macro_to_format
?? raise Exception("Unhandled macro '%s'" % macro)
Exception: Unhandled macro 'HWADDR_PRIx'


No idea. Nobody else reported a problem with this yet and the above error makes 
no
sense to me. Cc'd some tracetool related people who hopefully can hint what 
might be
wrong. Do you get error during build? What trace backend do you use?


I enable all trace backends for my integration testing.  The error is
thrown for the dtrace backend.


You can't use HWADDR_PRIx in trace-events because other non-qemu-based trace 
backends
have no awareness of it (as I also found out). For HWADDR_PRIx you need to use 
PRIx64


OK, does it compile for you if you change that to PRIx64? Should I submit 
a patch with that fix or do you want to make a patch?


Regards,
BALATON Zoltan



Re: [Qemu-devel] [PATCH v2 10/10] file-posix: Make auto-read-only dynamic

2019-03-11 Thread Markus Armbruster
Peter Krempa  writes:

> On Mon, Mar 11, 2019 at 15:10:36 -0500, Eric Blake wrote:
>> On 3/11/19 2:59 PM, Peter Krempa wrote:
>> 
>> >> auto-read-only was introduced in 3.1, at which point we intentionally
>> >> had sufficiently loose wording to permit (but not require) dynamic state
>> >> checking; so you are not breaking the interface.  On the other hand, is
>> >> libvirt going to have problems introspecting whether it can use
>> >> auto-read-only and get the dynamic behavior it needs?  Or is there
>> >> enough else in the way of libvirt's switch to -blockdev that it won't
>> >> attempt anything that needs auto-read-only without other 4.0 interfaces
>> >> anyway, at which point detecting the presence of the field (but not
>> >> whether the field has a guarantee of dynamic behavior) on 3.1 doesn't
>> >> matter?
>> > 
>> > I think we can use Stefan's capability detection mechanism he introduced
>> > for the migration with cache enabled for local files to add a flag for
>> > this as well.
>> 
>> Except I thought we decided that the most recent version of his QMP
>> changes was now fully-introspectible, thanks to using conditional
>> compilation.
>> 
>> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02510.html
>
> Oh, bummer, I missed that it was no longer needed. I still think it's
> worth adding that for future use once it will be necessary to detect
> that certain things were patched and require libvirt to change behaviour
> if that's the case.

We'll add it when we have a compelling use for it.

>> Well, that may prove to be a short-lived hiatus, if libvirt would
>> happily attempt to use qemu 3.1 and fail without some other
>> introspectible hook to know whether auto-read-only has required semantics.



Re: [Qemu-devel] [PULL 4/5] hw/display: Add basic ATI VGA emulation

2019-03-11 Thread Markus Armbruster
Mark Cave-Ayland  writes:

> On 11/03/2019 19:51, BALATON Zoltan wrote:
>
>> On Mon, 11 Mar 2019, Markus Armbruster wrote:
>>> Gerd Hoffmann  writes:
 From: BALATON Zoltan 

 At least two machines, the PPC mac99 and MIPS fulong2e, have an ATI
 gfx chip by default (Rage 128 Pro and M6/RV100 respectively) and
 guests running on these and the PMON2000 firmware of the fulong2e
 expect this to be available. Fortunately these are very similar chips
 so they can be mostly emulated in the same device model. This patch
 adds basic emulation of these ATI VGA chips.

 While this is incomplete and currently only enough to run the MIPS
 firmware and get framebuffer output with Linux, it allows the fulong2e
 board to work more like the real hardware and having it in QEMU in
 this state provides a way to experiment with it and allows others to
 contribute to improve it. It is compiled for all archs but only the
 fulong2e (which currently has no display output at all) is set to use
 it by default (in a separate patch).

 Signed-off-by: BALATON Zoltan 
 Acked-by: Aleksandar Markovic 
 Tested-by: Andrew Randrianasulu 
 Tested-by: Howard Spoelstra 
 Message-id:
 0b1b7c22873a6e37627261b04fb687412b25ff4f.1552152100.git.bala...@eik.bme.hu
 Signed-off-by: Gerd Hoffmann 
>>> [...]
 diff --git a/hw/display/trace-events b/hw/display/trace-events
 index 37d3264bb2e6..80993cc4d913 100644
 --- a/hw/display/trace-events
 +++ b/hw/display/trace-events
 @@ -138,3 +138,7 @@ vga_cirrus_write_blt(uint32_t offset, uint32_t val) 
 "offset
 0x%x, val 0x%x"
  sii9022_read_reg(uint8_t addr, uint8_t val) "addr 0x%02x, val 0x%02x"
  sii9022_write_reg(uint8_t addr, uint8_t val) "addr 0x%02x, val 0x%02x"
  sii9022_switch_mode(const char *mode) "mode: %s"
 +
 +# hw/display/ati*.c
 +ati_mm_read(unsigned int size, uint64_t addr, const char *name, uint64_t 
 val) "%u
 0x%"HWADDR_PRIx " %s -> 0x%"PRIx64
 +ati_mm_write(unsigned int size, uint64_t addr, const char *name, uint64_t 
 val)
 "%u 0x%"HWADDR_PRIx " %s <- 0x%"PRIx64
>>>
>>> Blows up for me:
>>>
>>> Traceback (most recent call last):
>>>  File "/work/armbru/qemu/scripts/tracetool.py", line 152, in 
>>>    main(sys.argv)
>>>  File "/work/armbru/qemu/scripts/tracetool.py", line 147, in main
>>>    binary=binary, probe_prefix=probe_prefix)
>>>  File "/work/armbru/qemu/scripts/tracetool/__init__.py", line 472, in 
>>> generate
>>>    tracetool.format.generate(events, format, backend, group)
>>>  File "/work/armbru/qemu/scripts/tracetool/format/__init__.py", line 85, in 
>>> generate
>>>    func(events, backend, group)
>>>  File "/work/armbru/qemu/scripts/tracetool/format/log_stap.py", line 121, 
>>> in generate
>>>    fmt_str = "%d@%d " + e.name + " " + c_fmt_to_stap(e.fmt) + "\\n"
>>>  File "/work/armbru/qemu/scripts/tracetool/format/log_stap.py", line 64, in
>>> c_fmt_to_stap
>>>    bits.append(c_macro_to_format(macro))
>>>  File "/work/armbru/qemu/scripts/tracetool/format/log_stap.py", line 36, in
>>> c_macro_to_format
>>>    raise Exception("Unhandled macro '%s'" % macro)
>>> Exception: Unhandled macro 'HWADDR_PRIx'
>> 
>> No idea. Nobody else reported a problem with this yet and the above error 
>> makes no
>> sense to me. Cc'd some tracetool related people who hopefully can hint what 
>> might be
>> wrong. Do you get error during build? What trace backend do you use?

I enable all trace backends for my integration testing.  The error is
thrown for the dtrace backend.

> You can't use HWADDR_PRIx in trace-events because other non-qemu-based trace 
> backends
> have no awareness of it (as I also found out). For HWADDR_PRIx you need to 
> use PRIx64
> instead. I have a vague recollection of someone posting a patch for 
> checkpatch to
> detect this - does it pass a local checkpatch run?

Only unrelated warnings:

$ scripts/checkpatch.pl 0001-hw-display-Add-basic-ATI-VGA-emulation.patch 
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#69: 
new file mode 100644

WARNING: line over 80 characters
#176: FILE: hw/display/ati.c:103:
+vbe_ioport_write_index(>vga, 0, 
VBE_DISPI_INDEX_Y_OFFSET);

total: 0 errors, 2 warnings, 1876 lines checked



Re: [Qemu-devel] [PATCH v2 10/11] hw/arm/virt: Add GED device configuration and build aml

2019-03-11 Thread Auger Eric
Hi Shameer,

On 3/8/19 12:42 PM, Shameer Kolothum wrote:
> This initializes the GED device with base memory and irq.
> It also configures ged memory hotplug event and builds the
> corresponding aml code.
> 
> But ged irq routing to Guest is not enabled and thus hotplug
> is not yet supported.
> 
> Signed-off-by: Shameer Kolothum 
> ---
>  default-configs/arm-softmmu.mak |  1 +
>  hw/arm/virt-acpi-build.c| 13 +
>  hw/arm/virt-acpi.c  |  4 
>  hw/arm/virt.c   | 22 ++
>  include/hw/arm/virt.h   |  4 
>  5 files changed, 44 insertions(+)
> 
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index b3bac25..7c442fd 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -168,3 +168,4 @@ CONFIG_MUSICPAL=y
>  CONFIG_MEM_DEVICE=y
>  CONFIG_DIMM=y
>  CONFIG_ACPI_MEMORY_HOTPLUG=y
> +CONFIG_ACPI_HW_REDUCED=y
requires a KConfig entry now
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 6cb7263..86f25ad 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -50,6 +50,18 @@
>  #define ARM_SPI_BASE 32
>  #define ACPI_POWER_BUTTON_DEVICE "PWRB"
>  
> +static void acpi_dsdt_add_ged(Aml *scope, VirtMachineState *vms)
> +{
> +int irq =  vms->irqmap[VIRT_ACPI_GED] + ARM_SPI_BASE;
> +
> +if (!vms->ged_events || !vms->ged_events_size) {
> +return;
> +}
> +
> +build_ged_aml(scope, "\\_SB."GED_DEVICE, irq, vms->ged_events,
> +  vms->ged_events_size, AML_SYSTEM_MEMORY);
> +}
> +
>  static void acpi_dsdt_add_memory_hotplug(Aml *scope, MachineState *ms)
>  {
>  uint32_t nr_mem = ms->ram_slots;
> @@ -758,6 +770,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>   */
>  scope = aml_scope("\\_SB");
>  acpi_dsdt_add_memory_hotplug(scope, MACHINE(vms));
> +acpi_dsdt_add_ged(scope, vms);
>  acpi_dsdt_add_cpus(scope, vms->smp_cpus);
>  acpi_dsdt_add_uart(scope, [VIRT_UART],
> (irqmap[VIRT_UART] + ARM_SPI_BASE));
> diff --git a/hw/arm/virt-acpi.c b/hw/arm/virt-acpi.c
> index 18ebe94..3b55c63 100644
> --- a/hw/arm/virt-acpi.c
> +++ b/hw/arm/virt-acpi.c
> @@ -31,6 +31,7 @@
>  typedef struct VirtAcpiState {
>  SysBusDevice parent_obj;
>  MemHotplugState memhp_state;
> +GEDState ged_state;
>  } VirtAcpiState;
>  
>  #define TYPE_VIRT_ACPI "virt-acpi"
> @@ -76,12 +77,15 @@ static void virt_device_realize(DeviceState *dev, Error 
> **errp)
>  {
>  VirtMachineState *vms = VIRT_MACHINE(qdev_get_machine());
>  const MemMapEntry *memmap = vms->memmap;
> +const int *irqmap = vms->irqmap;
>  VirtAcpiState *s = VIRT_ACPI(dev);
>  
>  if (s->memhp_state.is_enabled) {
>  acpi_memory_hotplug_init(get_system_memory(), OBJECT(dev),
>   >memhp_state,
>   memmap[VIRT_PCDIMM_ACPI].base);
> +acpi_ged_init(get_system_memory(), OBJECT(dev), >ged_state,
> +  memmap[VIRT_ACPI_GED].base, irqmap[VIRT_ACPI_GED]);
>  }
>  }
>  
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 9427f4f..352dbb1 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -134,6 +134,7 @@ static const MemMapEntry base_memmap[] = {
>  [VIRT_SECURE_UART] ={ 0x0904, 0x1000 },
>  [VIRT_SMMU] =   { 0x0905, 0x0002 },
>  [VIRT_PCDIMM_ACPI] ={ 0x0907, 0x0001 },
> +[VIRT_ACPI_GED] =   { 0x0908, 0x0001 },
>  [VIRT_MMIO] =   { 0x0a00, 0x0200 },
>  /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size 
> */
>  [VIRT_PLATFORM_BUS] =   { 0x0c00, 0x0200 },
> @@ -169,6 +170,7 @@ static const int a15irqmap[] = {
>  [VIRT_PCIE] = 3, /* ... to 6 */
>  [VIRT_GPIO] = 7,
>  [VIRT_SECURE_UART] = 8,
> +[VIRT_ACPI_GED] = 9,
>  [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>  [VIRT_GIC_V2M] = 48, /* ...to 48 + NUM_GICV2M_SPIS - 1 */
>  [VIRT_SMMU] = 74,/* ...to 74 + NUM_SMMU_IRQS - 1 */
> @@ -184,6 +186,25 @@ static const char *valid_cpus[] = {
>  ARM_CPU_TYPE_NAME("max"),
>  };
>  
> +static void virt_acpi_ged_conf(VirtMachineState *vms)
> +{
> +uint8_t events_size;
> +
> +/* GED events */
> +GedEvent events[] = {
> +{
> +.selector = ACPI_GED_IRQ_SEL_MEM,
> +.event= GED_MEMORY_HOTPLUG,
> +},
> +};
> +
> +events_size = ARRAY_SIZE(events);
> +
> +vms->ged_events = g_malloc0(events_size * sizeof(GedEvent));
> +memcpy(vms->ged_events, events, events_size * sizeof(GedEvent));
> +vms->ged_events_size = events_size;
> +}
can't you do the virt_acpi_ged_conf's job directly in virt_acpi_init
avoid this memcpy by using a pointer to a static const GedEvent
ged_events[]?
> +
>  static bool 

Re: [Qemu-devel] [PATCH v2 13/13] spapr/xive: fix device hotplug when VM is stopped

2019-03-11 Thread Cédric Le Goater
On 2/26/19 5:17 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 02:13:22PM +0100, Cédric Le Goater wrote:
>> Instead of switching off the sources, set their state to PENDING to
>> possibly catch a hotplug event occuring while the VM is stopped. At
>> resume, check the previous state and if an interrupt was queued,
>> generate a trigger.
> 
> First, I think it would be better to fold this fix into the patch
> introducing the state change handlers.> 
> Second, IIUC this would handle any instance of an irq being triggered
> while the VM is stopped. 

yes.

> Hotplug interrupts is one obvious case of that, but I'm not sure its 
> the only one.  

Do we really need to support device hotplug when the VM is stopped ? 
Is that a libvirt requirement ?  

> VFIO devices could interrupt while the VM is stopped, I think. 

If the guest has configured and mapped the IRQs, I would say yes. 

> Maybe even emulated devices
> depending on how their synchronization with the cpu run state works.

The console is one example.

> There might be other cases.  Does that sound right to you?

yes.

Supporting interrupts while a VM is stopped seems like a weird 
test scenario to me. Should we or should we not ? 

Thanks,

C.

>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/intc/spapr_xive_kvm.c | 22 +++---
>>  1 file changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
>> index 99a829fb3f60..64d160babb26 100644
>> --- a/hw/intc/spapr_xive_kvm.c
>> +++ b/hw/intc/spapr_xive_kvm.c
>> @@ -500,8 +500,16 @@ static void kvmppc_xive_change_state_handler(void 
>> *opaque, int running,
>>  if (running) {
>>  for (i = 0; i < xsrc->nr_irqs; i++) {
>>  uint8_t pq = xive_source_esb_get(xsrc, i);
>> -if (xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8)) != 
>> 0x1) {
>> -error_report("XIVE: IRQ %d has an invalid state", i);
>> +uint8_t old_pq;
>> +
>> +old_pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_00 + (pq << 8));
>> +
>> +/*
>> + * If an interrupt was queued (hotplug event) while VM was
>> + * stopped, generate a trigger.
>> + */
>> +if (pq == XIVE_ESB_RESET && old_pq == XIVE_ESB_QUEUED) {
>> +xive_esb_trigger(xsrc, i);
>>  }
>>  }
>>  
>> @@ -515,7 +523,15 @@ static void kvmppc_xive_change_state_handler(void 
>> *opaque, int running,
>>   * migration is in progress.
>>   */
>>  for (i = 0; i < xsrc->nr_irqs; i++) {
>> -uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_01);
>> +uint8_t pq = xive_esb_read(xsrc, i, XIVE_ESB_GET);
>> +
>> +/*
>> + * PQ is set to PENDING to possibly catch a hotplug event
>> + * occuring while the VM is stopped.
>> + */
>> +if (pq != XIVE_ESB_OFF) {
>> +pq = xive_esb_read(xsrc, i, XIVE_ESB_SET_PQ_10);
>> +}
>>  xive_source_esb_set(xsrc, i, pq);
>>  }
>>  
> 




Re: [Qemu-devel] ssh session with qemu-arm using busybox

2019-03-11 Thread Nick Kossifidis

Στις 2019-03-11 16:34, Pintu Agarwal έγραψε:

Hi,

I have a qemu-arm setup with busybox which I normally use to test my
kernel changes.
I use this command to boot it:

QEMU-ARM$
qemu-system-arm -M vexpress-a9 -m 1024M -kernel
../KERNEL/linux/arch/arm/boot/zImage -dtb
../KERNEL/linux/arch/arm/boot/dts/vexpress-v2p-ca9.dtb -initrd
rootfs.img.gz -append "console=ttyAMA0 root=/dev/ram rdinit=/sbin/init
ip=dhcp" -nographic -smp 4

This includes, my own custom kernel and rootfs build.
For rootfs I use busybox and create cpio image and use it as initrd.

But, every time, if I have to copy some files inside qemu, I need to
create create roofs image.
This is painful.

So, now I am exploring how to transfer files from my ubuntu pc to
qemu-session using "ssh".
I know this is possible, but I am still not successful, and bit lazy
to explore these.

I am sure, many of you people would have explored already "how to use
ssh over qemu" and found a easy method.
So, if anybody have easy setup please share with me.

I could see that after adding "ip=dhcp" I get the eth0 interface like 
this:

/ # ifconfig
eth0  Link encap:Ethernet  HWaddr 52:54:00:12:34:56
  inet addr:10.0.2.15  Bcast:10.0.2.255  Mask:255.255.255.0
  UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
  RX packets:2 errors:0 dropped:0 overruns:0 frame:0
  TX packets:2 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:1000
  RX bytes:1180 (1.1 KiB)  TX bytes:1180 (1.1 KiB)
  Interrupt:22

loLink encap:Local Loopback
  inet addr:127.0.0.1  Mask:255.0.0.0
  UP LOOPBACK RUNNING  MTU:65536  Metric:1
  RX packets:0 errors:0 dropped:0 overruns:0 frame:0
  TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
  collisions:0 txqueuelen:1000
  RX bytes:0 (0.0 B)  TX bytes:0 (0.0 B)

But I could not ping it from ubuntu PC.


Regards,
Pintu

___
linux-riscv mailing list
linux-ri...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv


You may use the hostfw argument on qemu, e.g...

-netdev user,id=unet,hostfwd=tcp::-:22 \
-net user \

and you 'll get guest's port 22 to be forwarded to hosts port , so 
you can do


ssh root@localhost:

from the host.

Regards,
Nick



Re: [Qemu-devel] [PATCH v5 0/10] qemu-binfmt-conf.sh

2019-03-11 Thread Unai Martinez-Corral
Unfortunately the subject of the first message in the v5 of this series
is wrong. I submitted '[PATCH v5 0/10] qemu-binfmt-conf.sh', but it
should have been '[PATCH v5 0/9] qemu-binfmt-conf.sh'.

All of the 9 patches have been properly submitted, but the patchset is
not detected as complete.

Do I need to resend all of them again?

I'm sorry for the inconvenience.



Re: [Qemu-devel] [PATCH v2 07/13] spapr/xive: fix migration of the XiveTCTX under TCG

2019-03-11 Thread Cédric Le Goater
On 2/26/19 2:02 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 02:13:16PM +0100, Cédric Le Goater wrote:
>> When the thread interrupt management state is retrieved from the KVM
>> VCPU, word2 is saved under the QEMU XIVE thread context to print out
>> the OS CAM line under the QEMU monitor.
>>
>> This breaks the migration of a TCG guest (and with KVM when
>> kernel_irqchip=off) because the matching algorithm of the presenter
>> relies on the OS CAM value. Fix with an extra reset of the thread
>> contexts to restore the expected value.
>>
>> Signed-off-by: Cédric Le Goater 
> 
> As noted elsewhere, I'm not sure this is the right approach to fixing
> this.  In any case this can be folded into the previous patch.

I have proposed an alternative in a response to :

 [PATCH v2 04/13] spapr/xive: add state synchronization with KVM 

C.


> 
>> ---
>>  hw/ppc/spapr_irq.c | 26 +-
>>  1 file changed, 25 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index 12ecca6264f3..3176098b9f7c 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -356,7 +356,31 @@ static void 
>> spapr_irq_cpu_intc_create_xive(sPAPRMachineState *spapr,
>>  
>>  static int spapr_irq_post_load_xive(sPAPRMachineState *spapr, int 
>> version_id)
>>  {
>> -return spapr_xive_post_load(spapr->xive, version_id);
>> +CPUState *cs;
>> +int ret;
>> +
>> +ret = spapr_xive_post_load(spapr->xive, version_id);
>> +if (ret) {
>> +return ret;
>> +}
>> +
>> +/*
>> + * When the states are collected from the KVM XIVE device, word2
>> + * of the XiveTCTX is set to print out the OS CAM line under the
>> + * QEMU monitor.
>> + *
>> + * This breaks the migration on a TCG guest (or on KVM with
>> + * kernel_irqchip=off) because the matching algorithm of the
>> + * presenter relies on the OS CAM value. Fix with an extra reset
>> + * of the thread contexts to restore the expected value.
>> + */
>> +CPU_FOREACH(cs) {
>> +PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +
>> +/* (TCG) Set the OS CAM line of the thread interrupt context. */
>> +spapr_xive_set_tctx_os_cam(spapr_cpu_state(cpu)->tctx);
>> +}
>> +return 0;
>>  }
>>  
>>  static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
> 




Re: [Qemu-devel] [PATCH v2 03/13] spapr/xive: activate KVM support

2019-03-11 Thread Cédric Le Goater
On 2/26/19 12:49 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 02:13:12PM +0100, Cédric Le Goater wrote:
>> All is in place for KVM now. State synchronization and migration will
>> come next.
> 
> As with the kernel side capability, this should be moved later in the
> series to avoid breaking bisections.

I am not sure to understand. At this stage of the patchset, the XIVE
exploitation mode is operational. We can not synchronise the state 
or migrate but it runs.

Should we move XIVE activation after the migration patch ? 

C. 

 
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/ppc/spapr_irq.c | 9 -
>>  1 file changed, 9 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
>> index 6e1c36dc62ca..1ad57582a403 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -263,19 +263,10 @@ sPAPRIrq spapr_irq_xics = {
>>  static void spapr_irq_init_xive(sPAPRMachineState *spapr, int nr_irqs,
>>  Error **errp)
>>  {
>> -MachineState *machine = MACHINE(spapr);
>>  uint32_t nr_servers = spapr_max_server_number(spapr);
>>  DeviceState *dev;
>>  int i;
>>  
>> -/* KVM XIVE device not yet available */
>> -if (kvm_enabled()) {
>> -if (machine_kernel_irqchip_required(machine)) {
>> -error_setg(errp, "kernel_irqchip requested. no KVM XIVE 
>> support");
>> -return;
>> -}
>> -}
>> -
>>  dev = qdev_create(NULL, TYPE_SPAPR_XIVE);
>>  qdev_prop_set_uint32(dev, "nr-irqs", nr_irqs);
>>  /*
> 




Re: [Qemu-devel] [PATCH v2 04/13] spapr/xive: add state synchronization with KVM

2019-03-11 Thread Cédric Le Goater
On 2/26/19 1:01 AM, David Gibson wrote:
> On Fri, Feb 22, 2019 at 02:13:13PM +0100, Cédric Le Goater wrote:
>> This extends the KVM XIVE device backend with 'synchronize_state'
>> methods used to retrieve the state from KVM. The HW state of the
>> sources, the KVM device and the thread interrupt contexts are
>> collected for the monitor usage and also migration.
>>
>> These get operations rely on their KVM counterpart in the host kernel
>> which acts as a proxy for OPAL, the host firmware. The set operations
>> will be added for migration support later.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  include/hw/ppc/spapr_xive.h |  8 
>>  include/hw/ppc/xive.h   |  1 +
>>  hw/intc/spapr_xive.c| 17 ---
>>  hw/intc/spapr_xive_kvm.c| 89 +
>>  hw/intc/xive.c  | 10 +
>>  5 files changed, 118 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
>> index 749c6cbc2c56..ebd65e7fe36b 100644
>> --- a/include/hw/ppc/spapr_xive.h
>> +++ b/include/hw/ppc/spapr_xive.h
>> @@ -44,6 +44,13 @@ typedef struct sPAPRXive {
>>  void  *tm_mmap;
>>  } sPAPRXive;
>>  
>> +/*
>> + * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
>> + * to the controller block id value. It can nevertheless be changed
>> + * for testing purpose.
>> + */
>> +#define SPAPR_XIVE_BLOCK_ID 0x0
>> +
>>  bool spapr_xive_irq_claim(sPAPRXive *xive, uint32_t lisn, bool lsi);
>>  bool spapr_xive_irq_free(sPAPRXive *xive, uint32_t lisn);
>>  void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor *mon);
>> @@ -74,5 +81,6 @@ void kvmppc_xive_set_queue_config(sPAPRXive *xive, uint8_t 
>> end_blk,
>>  void kvmppc_xive_get_queue_config(sPAPRXive *xive, uint8_t end_blk,
>>   uint32_t end_idx, XiveEND *end,
>>   Error **errp);
>> +void kvmppc_xive_synchronize_state(sPAPRXive *xive, Error **errp);
>>  
>>  #endif /* PPC_SPAPR_XIVE_H */
>> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
>> index 061d43fea24d..f3766fd881a2 100644
>> --- a/include/hw/ppc/xive.h
>> +++ b/include/hw/ppc/xive.h
>> @@ -431,5 +431,6 @@ void kvmppc_xive_source_reset_one(XiveSource *xsrc, int 
>> srcno, Error **errp);
>>  void kvmppc_xive_source_reset(XiveSource *xsrc, Error **errp);
>>  void kvmppc_xive_source_set_irq(void *opaque, int srcno, int val);
>>  void kvmppc_xive_cpu_connect(XiveTCTX *tctx, Error **errp);
>> +void kvmppc_xive_cpu_synchronize_state(XiveTCTX *tctx, Error **errp);
>>  
>>  #endif /* PPC_XIVE_H */
>> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
>> index 3db24391e31c..9f07567f4d78 100644
>> --- a/hw/intc/spapr_xive.c
>> +++ b/hw/intc/spapr_xive.c
>> @@ -40,13 +40,6 @@
>>  
>>  #define SPAPR_XIVE_NVT_BASE 0x400
>>  
>> -/*
>> - * The sPAPR machine has a unique XIVE IC device. Assign a fixed value
>> - * to the controller block id value. It can nevertheless be changed
>> - * for testing purpose.
>> - */
>> -#define SPAPR_XIVE_BLOCK_ID 0x0
>> -
>>  /*
>>   * sPAPR NVT and END indexing helpers
>>   */
>> @@ -153,6 +146,16 @@ void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor 
>> *mon)
>>  XiveSource *xsrc = >source;
>>  int i;
>>  
>> +if (kvm_irqchip_in_kernel()) {
>> +Error *local_err = NULL;
>> +
>> +kvmppc_xive_synchronize_state(xive, _err);
>> +if (local_err) {
>> +error_report_err(local_err);
>> +return;
>> +}
>> +}
>> +
>>  monitor_printf(mon, "  LSIN PQEISN CPU/PRIO EQ\n");
>>  
>>  for (i = 0; i < xive->nr_irqs; i++) {
>> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
>> index 6b50451b4f85..4b1ffb9835f9 100644
>> --- a/hw/intc/spapr_xive_kvm.c
>> +++ b/hw/intc/spapr_xive_kvm.c
>> @@ -60,6 +60,57 @@ static void kvm_cpu_enable(CPUState *cs)
>>  /*
>>   * XIVE Thread Interrupt Management context (KVM)
>>   */
>> +static void kvmppc_xive_cpu_get_state(XiveTCTX *tctx, Error **errp)
>> +{
>> +uint64_t state[4] = { 0 };
>> +int ret;
>> +
>> +ret = kvm_get_one_reg(tctx->cs, KVM_REG_PPC_NVT_STATE, state);
>> +if (ret != 0) {
>> +error_setg_errno(errp, errno,
>> + "XIVE: could not capture KVM state of CPU %ld",
>> + kvm_arch_vcpu_id(tctx->cs));
>> +return;
>> +}
>> +
>> +/* word0 and word1 of the OS ring. */
>> +*((uint64_t *) >regs[TM_QW1_OS]) = state[0];
>> +
>> +/*
>> + * KVM also returns word2 containing the OS CAM line which is
>> + * interesting to print out in the QEMU monitor.
>> + */
>> +*((uint64_t *) >regs[TM_QW1_OS + TM_WORD2]) = state[1];
> 
> As mentioned elsewhere, it is interesting for debugging, but doesn't
> seem to really match the guest visible CAM state, 

The guest is not allowed to see these registers in the TIMA OS page 
and we are not violating the XIVE architecture. That is where 

Re: [Qemu-devel] [PATCH v2 10/10] file-posix: Make auto-read-only dynamic

2019-03-11 Thread Peter Krempa
On Mon, Mar 11, 2019 at 15:10:36 -0500, Eric Blake wrote:
> On 3/11/19 2:59 PM, Peter Krempa wrote:
> 
> >> auto-read-only was introduced in 3.1, at which point we intentionally
> >> had sufficiently loose wording to permit (but not require) dynamic state
> >> checking; so you are not breaking the interface.  On the other hand, is
> >> libvirt going to have problems introspecting whether it can use
> >> auto-read-only and get the dynamic behavior it needs?  Or is there
> >> enough else in the way of libvirt's switch to -blockdev that it won't
> >> attempt anything that needs auto-read-only without other 4.0 interfaces
> >> anyway, at which point detecting the presence of the field (but not
> >> whether the field has a guarantee of dynamic behavior) on 3.1 doesn't
> >> matter?
> > 
> > I think we can use Stefan's capability detection mechanism he introduced
> > for the migration with cache enabled for local files to add a flag for
> > this as well.
> 
> Except I thought we decided that the most recent version of his QMP
> changes was now fully-introspectible, thanks to using conditional
> compilation.
> 
> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg02510.html

Oh, bummer, I missed that it was no longer needed. I still think it's
worth adding that for future use once it will be necessary to detect
that certain things were patched and require libvirt to change behaviour
if that's the case.

> Well, that may prove to be a short-lived hiatus, if libvirt would
> happily attempt to use qemu 3.1 and fail without some other
> introspectible hook to know whether auto-read-only has required semantics.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 





signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 00/10] file-posix: Make auto-read-only dynamic

2019-03-11 Thread Peter Krempa
On Mon, Mar 11, 2019 at 17:50:07 +0100, Kevin Wolf wrote:
> We introduced the auto-read-only option to fix the problem that block
> jobs that reopen a backing file read-write don't work any more when all
> nodes are created individually with -blockdev. The reason is that
> bs->file of these backing files doesn't inherit the read-only option
> from the format layer node any more if it's created separately.
> 
> The way auto-read-only was designed to fix this is that it just always
> opens the file node read-write if it can, so reopening the format layer
> node is enough to make the backing file writable when necessary.
> 
> This works in principle, but not when libvirt uses sVirt: Then QEMU
> doesn't even have the permissions to open the image file read-write
> until libvirt performs an operation where write access is needed.
> 
> This series changes auto-read-only so that it works dynamically and
> automatically reopens the file read-only or read-write depending on the
> permissions that users attached to the node requested.
> 
> See also: https://bugzilla.redhat.com/show_bug.cgi?id=1685989
> 
> v2:
> - Added test for and fixed snapshot=on,read-only=on regression [Berto]
> - Added test for and fixed commit regression [Peter]

I've quickly tested it with non-active commit and it works in this
scenario as expected.

I'll give it a bit more thorough test in the morning.


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH v5 9/9] qemu-binfmt-conf.sh: add --test

2019-03-11 Thread Unai Martinez-Corral
Signed-off-by: Unai Martinez-Corral 
---
 scripts/qemu-binfmt-conf.sh | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/scripts/qemu-binfmt-conf.sh b/scripts/qemu-binfmt-conf.sh
index 382bdaabfe..b750f60ef5 100755
--- a/scripts/qemu-binfmt-conf.sh
+++ b/scripts/qemu-binfmt-conf.sh
@@ -197,8 +197,7 @@ Options and associated environment variables:

 Argument Env-variable Description
 TARGETS  QEMU_TARGETS A single arch name or a list of them 
(see all names below);
-  if empty, configure/clear all known 
targets;
-  if 'NONE', no interpreter is configured.
+  if empty, configure/clear all known 
targets.
 -h|--help display this usage
 -Q|--path PATH   QEMU_PATHset path to qemu interpreter(s)
 -F|--suffix SUFFIX   QEMU_SUFFIX  add a suffix to the default interpreter 
name
@@ -208,6 +207,8 @@ TARGETS  QEMU_TARGETS A single arch name or 
a list of them (see
   to the binary to interpret
 -r|--clear   QEMU_CLEAR   (yes) remove registered interpreters for 
target TARGETS;
   then exit.
+-t|--testQEMU_TEST(yes) test the setup with the provided 
arguments, but do not
+  configure any of the interpreters.
 -e|--exportdir PATH  DEBIANDIRdefine where to write configuration files
  SYSTEMDDIR
 -s|--systemd  don't write into /proc, generate file(s) 
for
@@ -221,6 +222,7 @@ QEMU_SUFFIX=$QEMU_SUFFIX
 QEMU_PERSISTENT=$QEMU_PERSISTENT
 QEMU_CREDENTIAL=$QEMU_CREDENTIAL
 QEMU_CLEAR=$QEMU_CLEAR
+QEMU_TEST=$QEMU_TEST

 To import templates with update-binfmts, use :

@@ -319,9 +321,6 @@ qemu_set_binfmts() {

 # reduce the list of target interpreters to those given in the CLI
 [ $# -eq 0 ] && targets="${QEMU_TARGETS:-}" || targets="$@"
-if [ "x$targets" = "xNONE" ] ; then
-  return
-fi
 qemu_check_target_list $targets

 # register the interpreter for each target except for the native one
@@ -379,12 +378,16 @@ QEMU_SUFFIX="${QEMU_SUFFIX:-}"
 QEMU_PERSISTENT="${QEMU_PERSISTENT:-no}"
 QEMU_CREDENTIAL="${QEMU_CREDENTIAL:-no}"
 QEMU_CLEAR="${QEMU_CLEAR:-no}"
+QEMU_TEST="${QEMU_TEST:-no}"

-options=$(getopt -o rdsQ:S:e:hcp -l 
clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent -- 
"$@")
+options=$(getopt -o trdsQ:S:e:hcp -l 
test,clear,debian,systemd,path:,suffix:,exportdir:,help,credential,persistent 
-- "$@")
 eval set -- "$options"

 while true ; do
 case "$1" in
+-t|--test)
+QEMU_TEST="yes"
+;;
 -r|--clear)
 QEMU_CLEAR="yes"
 ;;
@@ -438,4 +441,8 @@ if [ "x$QEMU_CLEAR" = "xyes" ] ; then
 exit
 fi

+if [ "x$QEMU_TEST" = "xyes" ] ; then
+BINFMT_SET=:
+fi
+
 qemu_set_binfmts "$@"
--
2.21.0




  1   2   3   4   5   6   >