Re: [Qemu-devel] [PATCH] target/openrisc: Fix LGPL information in the file headers

[Thomas Huth]

On 13/02/2019 16.59, Thomas Huth wrote:
> It's either "GNU *Library* General Public License version 2" or "GNU
> Lesser General Public License version *2.1*", but there was no "version
> 2.0" of the "Lesser" license. So assume that version 2.1 is meant here.
> 
> Signed-off-by: Thomas Huth 
> ---
>  hw/openrisc/cputimer.c   | 2 +-
>  hw/openrisc/openrisc_sim.c   | 2 +-
>  hw/openrisc/pic_cpu.c| 2 +-
>  linux-user/openrisc/target_cpu.h | 2 +-
>  linux-user/openrisc/target_structs.h | 2 +-
>  target/openrisc/cpu.h| 2 +-
>  target/openrisc/exception.c  | 2 +-
>  target/openrisc/exception_helper.c   | 2 +-
>  target/openrisc/fpu_helper.c | 2 +-
>  target/openrisc/insns.decode | 2 +-
>  target/openrisc/interrupt.c  | 2 +-
>  target/openrisc/machine.c| 2 +-
>  target/openrisc/mmu.c| 2 +-
>  13 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/openrisc/cputimer.c b/hw/openrisc/cputimer.c
> index 850f887..fe95efc 100644
> --- a/hw/openrisc/cputimer.c
> +++ b/hw/openrisc/cputimer.c
> @@ -7,7 +7,7 @@
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
>   * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> + * version 2.1 of the License, or (at your option) any later version.
>   *
>   * This library is distributed in the hope that it will be useful,
>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
> diff --git a/hw/openrisc/openrisc_sim.c b/hw/openrisc/openrisc_sim.c
> index 7d3b734..0a906d8 100644
> --- a/hw/openrisc/openrisc_sim.c
> +++ b/hw/openrisc/openrisc_sim.c
> @@ -7,7 +7,7 @@
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
>   * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> + * version 2.1 of the License, or (at your option) any later version.
>   *
>   * This library is distributed in the hope that it will be useful,
>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
> diff --git a/hw/openrisc/pic_cpu.c b/hw/openrisc/pic_cpu.c
> index 569b443..2f53cfc 100644
> --- a/hw/openrisc/pic_cpu.c
> +++ b/hw/openrisc/pic_cpu.c
> @@ -7,7 +7,7 @@
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
>   * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> + * version 2.1 of the License, or (at your option) any later version.
>   *
>   * This library is distributed in the hope that it will be useful,
>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
> diff --git a/linux-user/openrisc/target_cpu.h 
> b/linux-user/openrisc/target_cpu.h
> index d1ea450..32ff135 100644
> --- a/linux-user/openrisc/target_cpu.h
> +++ b/linux-user/openrisc/target_cpu.h
> @@ -6,7 +6,7 @@
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
>   * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> + * version 2.1 of the License, or (at your option) any later version.
>   *
>   * This library is distributed in the hope that it will be useful,
>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
> diff --git a/linux-user/openrisc/target_structs.h 
> b/linux-user/openrisc/target_structs.h
> index afbb7ad..e98e2bc 100644
> --- a/linux-user/openrisc/target_structs.h
> +++ b/linux-user/openrisc/target_structs.h
> @@ -6,7 +6,7 @@
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
>   * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> + * version 2.1 of the License, or (at your option) any later version.
>   *
>   * This library is distributed in the hope that it will be useful,
>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
> diff --git a/target/openrisc/cpu.h b/target/openrisc/cpu.h
> index f1b31bc..c104a4b 100644
> --- a/target/openrisc/cpu.h
> +++ b/target/openrisc/cpu.h
> @@ -6,7 +6,7 @@
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
>   * License as published by the Free Software Foundation; either
> - * version 2 of the License, or (at your option) any later version.
> + * version 2.1 of the License, or (at your option) any later version.
>   *
>   * This library is distributed in the hope that it will be useful,
>   * 

Re: [Qemu-devel] [PATCH] hw/i2c/smbus_ich9: Fix the confusing contributions-after-2012 statement

[Thomas Huth]

On 29/03/2019 09.42, Thomas Huth wrote:
> On 06/02/2019 17.43, Thomas Huth wrote:
>> The license information in this file is rather confusing. The text
>> declares LGPL first, but then says that contributions after Jan 2012
>> are licensed under the GPL instead. How should the average user who
>> just downloaded the release tarball know which part is now GPL and
>> which is LGPL? Also, as far as I can see, the file has been added to
>> QEMU *after* January in 2012, so the whole file should be GPL by
>> default instead.
>>
>> Furthermore, looking at the text of the LGPL (see COPYING.LIB in the
>> top directory), the license clearly states in section "3." that one
>> should rather replace the license information in such a case instead.
>> Thus let's clean up the confusing statements and use the proper GPL
>> text only.
>>
>> While we're at it, also remove the comment about acpi.c, since that
>> file does not exist under this name in the QEMU tree anymore.
>>
>> Signed-off-by: Thomas Huth 
>> ---
>>  hw/i2c/smbus_ich9.c | 21 -
>>  1 file changed, 8 insertions(+), 13 deletions(-)
>>
>> diff --git a/hw/i2c/smbus_ich9.c b/hw/i2c/smbus_ich9.c
>> index 2a8b49e..484aef5 100644
>> --- a/hw/i2c/smbus_ich9.c
>> +++ b/hw/i2c/smbus_ich9.c
>> @@ -6,23 +6,18 @@
>>   *   VA Linux Systems Japan K.K.
>>   * Copyright (C) 2012 Jason Baron 
>>   *
>> - * This is based on acpi.c, but heavily rewritten.
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>>   *
>> - * This library is free software; you can redistribute it and/or
>> - * modify it under the terms of the GNU Lesser General Public
>> - * License version 2 as published by the Free Software Foundation.
>> - *
>> - * This library is distributed in the hope that it will be useful,
>> + * This program is distributed in the hope that it will be useful,
>>   * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>   * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> - * Lesser General Public License for more details.
>> - *
>> - * You should have received a copy of the GNU Lesser General Public
>> - * License along with this library; if not, see 
>> 
>> - *
>> - * Contributions after 2012-01-13 are licensed under the terms of the
>> - * GNU GPL, version 2 or (at your option) any later version.
>> + * General Public License for more details.
>>   *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, see 
>>   */
>>  #include "qemu/osdep.h"
>>  #include "hw/hw.h"
>>
> 
> Ping?

Ping^2

 Thomas

[Qemu-devel] [PATCH] tests/Makefile: Remove unused test-obj-y variable

[Thomas Huth]

I recently noticed that test-obj-y contains a file called
tests/check-block-qtest.o which simply does not belong to any .c
file and thus wondered why this is not causing any trouble.
Well, if I get the Makefile magic right, test-obj-y is not really
used for anything - and "make check" still works fine if we simply
remove it.

Signed-off-by: Thomas Huth 
---
 tests/Makefile.include | 19 ---
 1 file changed, 19 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7c8b9c84b2..dfc4b7746f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -482,25 +482,6 @@ GENERATED_FILES += tests/test-qapi-types.h \
tests/test-qapi-events-sub-sub-module.h \
tests/test-qapi-introspect.h
 
-test-obj-y = tests/check-qnum.o tests/check-qstring.o tests/check-qdict.o \
-   tests/check-qlist.o tests/check-qnull.o tests/check-qobject.o \
-   tests/check-qjson.o tests/check-qlit.o \
-   tests/check-block-qtest.o \
-   tests/test-coroutine.o tests/test-string-output-visitor.o \
-   tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \
-   tests/test-clone-visitor.o \
-   tests/test-qobject-input-visitor.o \
-   tests/test-qmp-cmds.o tests/test-visitor-serialization.o \
-   tests/test-x86-cpuid.o tests/test-mul64.o tests/test-int128.o \
-   tests/test-opts-visitor.o tests/test-qmp-event.o \
-   tests/rcutorture.o tests/test-rcu-list.o \
-   tests/test-rcu-simpleq.o \
-   tests/test-rcu-tailq.o \
-   tests/test-qdist.o tests/test-shift128.o \
-   tests/test-qht.o tests/qht-bench.o tests/test-qht-par.o \
-   tests/atomic_add-bench.o tests/atomic64-bench.o
-
-$(test-obj-y): QEMU_INCLUDES += -Itests
 QEMU_CFLAGS += -I$(SRC_PATH)/tests
 
 
-- 
2.21.0

Re: [Qemu-devel] [PATCH 0/2] buffer and delay backup COW write operation

[Liang Li]

On Tue, Apr 30, 2019 at 10:35:32AM +, Vladimir Sementsov-Ogievskiy wrote:
> 28.04.2019 13:01, Liang Li wrote:
> > If the backup target is a slow device like ceph rbd, the backup
> > process will affect guest BLK write IO performance seriously,
> > it's cause by the drawback of COW mechanism, if guest overwrite the
> > backup BLK area, the IO can only be processed after the data has
> > been written to backup target.
> > The impact can be relieved by buffering data read from backup
> > source and writing to backup target later, so the guest BLK write
> > IO can be processed in time.
> > Data area with no overwrite will be process like before without
> > buffering, in most case, we don't need a very large buffer.
> > 
> > An fio test was done when the backup was going on, the test resut
> > show a obvious performance improvement by buffering.
> 
> Hi Liang!
> 
> Good thing. Something like this I've briefly mentioned in my KVM Forum 2018
> report as "RAM Cache", and I'd really prefer this functionality to be a 
> separate
> filter, instead of complication of backup code. Further more, write notifiers
> will go away from backup code, after my backup-top series merged.
> 
> v5: https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg06211.html
> and separated preparing refactoring v7: 
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg04813.html
> 
> RAM Cache should be a filter driver, with an in-memory buffer(s) for data 
> written to it
> and with ability to flush data to underlying backing file.
> 
> Also, here is another approach for the problem, which helps if guest writing 
> activity
> is really high and long and buffer will be filled and performance will 
> decrease anyway:
> 
> 1. Create local temporary image, and COWs will go to it. (previously 
> considered on list, that we should call
> these backup operations issued by guest writes CBW = copy-before-write, as 
> copy-on-write
> is generally another thing, and using this term in backup is confusing).
> 
> 2. We also set original disk as a backing for temporary image, and start 
> another backup from
> temporary to real target.
> 
> This scheme is almost possible now, you need to start backup(sync=none) from 
> source to temp,
> to do [1]. Some patches are still needed to allow such scheme. I didn't send 
> them, as I want
> my other backup patches go first anyway. But I can. On the other hand if 
> approach with in-memory
> buffer works for you it may be better.
> 
> Also, I'm not sure for now, should we really do this thing through two backup 
> jobs, or we just
> need one separate backup-top filter and one backup job without filter, or we 
> need an additional
> parameter for backup job to set cache-block-node.
> 

Hi Vladimir,

   Thanks for your valuable information. I didn't notice that you are already 
working on
this,  so my patch will conflict with your work. We have thought about the way 
[2] and
give it up because it would affect local storage performance.
   I have read your slice in KVM Forum 2018 and the related patches, your 
solution can
help to solve the issues in backup. I am not sure if the "RAM cache" is a qcow2 
file in
RAM? if so, your implementation will free the RAM space occupied by BLK data 
once it's
written to the far target in time? or we may need a large cache to make things 
work.
   Two backup jobs seems complex and not user friendly, is it possible to make 
my patch
cowork with CBW?

Liang

Re: [Qemu-devel] [PATCH] hw/timer: Compile devices not target-dependent as common objects

[Thomas Huth]

On 05/05/2019 20.07, Philippe Mathieu-Daudé wrote:
> All these devices do not contain any target-specific code. While
> most of them are arch-specific, they are shared between different
> targets of the same arch family (ARM and AArch64, MIPS32/MIPS64,
> multiple endianess, ...).
> Put them into common-obj-y to compile them once for all targets.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/timer/Makefile.objs | 22 +++---
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
> index 0e9a4530f84..a92e22938cb 100644
> --- a/hw/timer/Makefile.objs
> +++ b/hw/timer/Makefile.objs
> @@ -25,20 +25,20 @@ common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
>  common-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-rtc.o
>  common-obj-$(CONFIG_NRF51_SOC) += nrf51_timer.o
>  
> -obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o
> -obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
> -obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
> -obj-$(CONFIG_EXYNOS4) += exynos4210_rtc.o
> -obj-$(CONFIG_OMAP) += omap_gptimer.o
> -obj-$(CONFIG_OMAP) += omap_synctimer.o
> -obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
> -obj-$(CONFIG_SH4) += sh_timer.o
> -obj-$(CONFIG_DIGIC) += digic-timer.o
> -obj-$(CONFIG_MIPS_CPS) += mips_gictimer.o
> +common-obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o
> +common-obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
> +common-obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
> +common-obj-$(CONFIG_EXYNOS4) += exynos4210_rtc.o
> +common-obj-$(CONFIG_OMAP) += omap_gptimer.o
> +common-obj-$(CONFIG_OMAP) += omap_synctimer.o
> +common-obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
> +common-obj-$(CONFIG_SH4) += sh_timer.o
> +common-obj-$(CONFIG_DIGIC) += digic-timer.o
> +common-obj-$(CONFIG_MIPS_CPS) += mips_gictimer.o
>  
>  obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
>  
> -obj-$(CONFIG_ALLWINNER_A10_PIT) += allwinner-a10-pit.o
> +common-obj-$(CONFIG_ALLWINNER_A10_PIT) += allwinner-a10-pit.o
>  
>  common-obj-$(CONFIG_STM32F2XX_TIMER) += stm32f2xx_timer.o
>  common-obj-$(CONFIG_ASPEED_SOC) += aspeed_timer.o

I've checked

 grep -r TARGET hw/timer/

and this looks good to me, so:

Reviewed-by: Thomas Huth 

By the way, I was surprised to see TARGET_FMT_plx there, since I first
thought that this would be a target-specific define, too. But apparently
it is not. Very confusing. I'd suggest to rename that to HWADDR_FMT_plx
instead, what do you think?

 Thomas

Re: [Qemu-devel] [PATCH] hw/display/cirrus_vga: Remove unused include

[Thomas Huth]

On 06/05/2019 00.56, Philippe Mathieu-Daudé wrote:
> Commit ce3cf70edaaf split the ISA device out of the PCI one,
> but forgot to remove the "hw/loader.h" header inclusion (the ISA
> device calls rom_add_vga()).  Remove the now unused include.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/display/cirrus_vga.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
> index a0e71469f4d..f67df3c1f6f 100644
> --- a/hw/display/cirrus_vga.c
> +++ b/hw/display/cirrus_vga.c
> @@ -33,7 +33,6 @@
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
>  #include "ui/pixel_ops.h"
> -#include "hw/loader.h"
>  #include "cirrus_vga_internal.h"

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH 1/1] MAINTAINERS: Add an entry for the Parallel NOR Flash devices

[Thomas Huth]

On 06/05/2019 00.47, Philippe Mathieu-Daudé wrote:
> Step in to maintain it, since I have some familiarity with
> the technology.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  MAINTAINERS | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 66ddbda9c95..633f6315536 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1391,6 +1391,13 @@ F: include/hw/net/
>  F: tests/virtio-net-test.c
>  T: git https://github.com/jasowang/qemu.git net
>  
> +Parallel NOR Flash devices
> +M: Philippe Mathieu-Daudé 
> +T: git https://gitlab.com/philmd/qemu.git pflash-next
> +S: Maintained
> +F: hw/block/pflash_cfi*.c
> +F: include/hw/block/flash.h

FWIW:

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v4 01/10] block/pflash_cfi02: Add test for supported commands

[Thomas Huth]

On 26/04/2019 18.26, Stephen Checkoway wrote:
> Test the AMD command set for parallel flash chips. This test uses an
> ARM musicpal board with a pflash drive to test the following list of
> currently-supported commands.
> - Autoselect
> - CFI
> - Sector erase
> - Chip erase
> - Program
> - Unlock bypass
> - Reset
> 
> Signed-off-by: Stephen Checkoway 
> ---
>  tests/Makefile.include|   2 +
>  tests/pflash-cfi02-test.c | 225 ++
>  2 files changed, 227 insertions(+)
>  create mode 100644 tests/pflash-cfi02-test.c

Acked-by: Thomas Huth 

Re: [Qemu-devel] [PATCH 2/2] ppc: Add dump-stack implementation

[David Gibson]

On Thu, May 02, 2019 at 01:47:32PM +1000, Alexey Kardashevskiy wrote:
> 
> 
> On 02/05/2019 10:43, David Gibson wrote:
> > On Wed, May 01, 2019 at 07:48:48PM +1000, Alexey Kardashevskiy wrote:
> >>
> >>
> >> On 01/05/2019 15:35, Suraj Jitindar Singh wrote:
> >>> The monitor function dump-stack is used to dump the stack for a cpu.
> >>> This can be useful for debugging purposes when the stack cannot be
> >>> dumped by another means.
> >>>
> >>> Add a ppc implementation ppc_cpu_dump_stack().
> >>> The stack pointer is stored in R1 with the back pointer at offset 0 and
> >>> the link register at offset 2.
> >>> Also dump the registers from the stack frame if the marker "regshere" is
> >>> found.
> >>
> >> Is this a Linux only marker? ABI does not mentioned this.
> >>
> >>> This only dumps the kernel stack, stopping if a non-kernel address is
> >>> found in the stack.
> >>
> >> Why enforce this limit?
> > 
> > It's also making a Linux specific assumption about addresses.
> 
> It does not have to if it used ppc_cpu_get_phys_page_debug(), the only
> linux specific left is that "regshere" marker, otherwise it could work
> for AIX or FreeBSD (obviously without the exception frame).

Sorry, I thought this was relying on the fact that Linux put its
linear mapping at 0xc0..., but I realized I was misreading what was
just the masking of the top two real mode address bits which is in the
hardware.

-- 
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] virtfs: Add missing "id" parameter in documentation

[Thomas Huth]

On 05/05/2019 20.32, Greg Kurz wrote:
> Hi Thomas,
> 
> Thanks for the janitoring :)
> 
> On Sun,  5 May 2019 16:45:27 +0200
> Thomas Huth  wrote:
> 
>> ... and remove the square brackets from "path" and "security_model",
>> since these parameters are not optional.
>>
> 
> Well this is only true when fsdriver == local, but the other fs drivers,
> ie. proxy and synth, don't need it at all.

Ok, then this is wrong in the output of "--help" instead.

> Each driver has its own set of
> options actually. This should better be described with separate lines IMHO.
> 
> Also, it should be stated that "id" relates to the fs backend, ie. it
> belongs to the -fsdev "id" space, not to the device that gets exposed
> to the guest.

Hmm, maybe it would be better if you do this patch, since you've
definitely got way more knowledge here than I do... Otherwise, I can
have a try, but it might take a while till I get back to this...

 Thomas

Re: [Qemu-devel] [PATCH 0/3] hw/ppc/40p: Move the MC146818 RTC to the board where it belongs

[David Gibson]

On Sun, May 05, 2019 at 05:28:36PM +0200, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series is to properly do the fix sent by Artyom here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02264.html
> 
> There is no RTC on the i82378, move it to the board code,
> set the base year there.

Applied to ppc-for-4.1, although see comment for a possible followup.

> 
> Regards,
> 
> Phil.
> 
> Artyom Tarasenko (1):
>   hw/ppc/40p: use 1900 as a base year
> 
> Philippe Mathieu-Daudé (2):
>   hw/ppc/prep: use TYPE_MC146818_RTC instead of a hardcoded string
>   hw/ppc/40p: Move the MC146818 RTC to the board where it belongs
> 
>  hw/isa/i82378.c | 4 
>  hw/ppc/prep.c   | 7 ++-
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 

-- 
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] [QEMU-PPC] [PATCH] target/ppc: Add ibm, purr and ibm, spurr device-tree properties

[David Gibson]

On Mon, May 06, 2019 at 11:48:03AM +1000, Suraj Jitindar Singh wrote:
> The ibm,purr and ibm,spurr device tree properties are used to indicate
> that the processor implements the Processor Utilisation of Resources
> Register (PURR) and Scaled Processor Utilisation of Resources Registers
> (SPURR), respectively. Each property has a single value which represents
> the level of architecture supported. A value of 1 for ibm,purr means
> support for the version of the PURR defined in book 3 in version 2.02 of
> the architecture. A value of 1 for ibm,spurr means support for the
> version of the SPURR defined in version 2.05 of the architecture.
> 
> Add these properties for all processors for which the PURR and SPURR
> registers are generated.

So.. what does the current empty property mean?  Is it just wrong by
spec, or does it actually mean something incorrect?

> 
> Signed-off-by: Suraj Jitindar Singh 
> ---
>  hw/ppc/spapr.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 2ef3ce4362..8580a8dc67 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -500,7 +500,10 @@ static void spapr_populate_cpu_dt(CPUState *cs, void 
> *fdt, int offset,
>  _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
>  
>  if (env->spr_cb[SPR_PURR].oea_read) {
> -_FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0)));
> +_FDT((fdt_setprop_cell(fdt, offset, "ibm,purr", 1)));
> +}
> +if (env->spr_cb[SPR_SPURR].oea_read) {
> +_FDT((fdt_setprop_cell(fdt, offset, "ibm,spurr", 1)));
>  }
>  
>  if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) {

-- 
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 1/3] hw/ppc/prep: use TYPE_MC146818_RTC instead of a hardcoded string

[David Gibson]

On Sun, May 05, 2019 at 05:28:37PM +0200, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé 

Certainly a good fix, but looks like there's places in
hw/timer/mc146818rtc.c itself and in vl.c which could also do with
this.

> ---
>  hw/ppc/prep.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index b7f459d4754..ebee3211480 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -601,7 +601,7 @@ static int prep_set_cmos_checksum(DeviceState *dev, void 
> *opaque)
>  uint16_t checksum = *(uint16_t *)opaque;
>  ISADevice *rtc;
>  
> -if (object_dynamic_cast(OBJECT(dev), "mc146818rtc")) {
> +if (object_dynamic_cast(OBJECT(dev), TYPE_MC146818_RTC)) {
>  rtc = ISA_DEVICE(dev);
>  rtc_set_memory(rtc, 0x2e, checksum & 0xff);
>  rtc_set_memory(rtc, 0x3e, checksum & 0xff);

-- 
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 v2 2/2] drm/i915/gvt: export mdev device version to sysfs for Intel vGPU

[Zhenyu Wang]

On 2019.05.05 21:51:02 -0400, Yan Zhao wrote:
> This feature implements the version attribute for Intel's vGPU mdev
> devices.
> 
> version attribute is rw.
> It's used to check device compatibility for two mdev devices.
> version string format and length are private for vendor driver. vendor
> driver is able to define them freely.
> 
> For Intel vGPU of gen8 and gen9, the mdev device version
> consists of 3 fields: "vendor id" + "device id" + "mdev type".
> 
> Reading from a vGPU's version attribute, a string is returned in below
> format: --. e.g.
> 8086-193b-i915-GVTg_V5_2.
> 
> Writing a string to a vGPU's version attribute will trigger GVT to check
> whether a vGPU identified by the written string is compatible with
> current vGPU owning this version attribute. errno is returned if the two
> vGPUs are incompatible. The length of written string is returned in
> compatible case.
> 
> For other platforms, and for GVT not supporting vGPU live migration
> feature, errnos are returned when read/write of mdev devices' version
> attributes.
> 
> For old GVT versions where no version attributes exposed in sysfs, it is
> regarded as not supporting vGPU live migration.
> 
> For future platforms, besides the current 2 fields in vendor proprietary
> part, more fields may be added to identify Intel vGPU well for live
> migration purpose.
> 
> v2:
> 1. removed 32 common part of version string
> (Alex Williamson)
> 2. do not register version attribute for GVT not supporting live
> migration.(Cornelia Huck)
> 3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
> incompatible. (Cornelia Huck)
> 
> Cc: Alex Williamson 
> Cc: Erik Skultety 
> Cc: "Dr. David Alan Gilbert" 
> Cc: Cornelia Huck 
> Cc: "Tian, Kevin" 
> Cc: Zhenyu Wang 
> Cc: "Wang, Zhi A" 
> c: Neo Jia 
> Cc: Kirti Wankhede 
> 
> Signed-off-by: Yan Zhao 
> ---
>  drivers/gpu/drm/i915/gvt/Makefile |  2 +-
>  drivers/gpu/drm/i915/gvt/device_version.c | 87 +++
>  drivers/gpu/drm/i915/gvt/gvt.c| 51 +
>  drivers/gpu/drm/i915/gvt/gvt.h|  6 ++
>  4 files changed, 145 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c
> 
> diff --git a/drivers/gpu/drm/i915/gvt/Makefile 
> b/drivers/gpu/drm/i915/gvt/Makefile
> index 271fb46d4dd0..54e209a23899 100644
> --- a/drivers/gpu/drm/i915/gvt/Makefile
> +++ b/drivers/gpu/drm/i915/gvt/Makefile
> @@ -3,7 +3,7 @@ GVT_DIR := gvt
>  GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o 
> firmware.o \
>   interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
>   execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o 
> debugfs.o \
> - fb_decoder.o dmabuf.o page_track.o
> + fb_decoder.o dmabuf.o page_track.o device_version.o
>  
>  ccflags-y+= -I$(src) -I$(src)/$(GVT_DIR)
>  i915-y   += $(addprefix $(GVT_DIR)/, 
> $(GVT_SOURCE))
> diff --git a/drivers/gpu/drm/i915/gvt/device_version.c 
> b/drivers/gpu/drm/i915/gvt/device_version.c
> new file mode 100644
> index ..bd4cdcbdba95
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gvt/device_version.c
> @@ -0,0 +1,87 @@
> +/*
> + * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
> THE
> + * SOFTWARE.
> + *
> + * Authors:
> + *Yan Zhao 
> + */
> +#include 
> +#include "i915_drv.h"
> +
> +static bool is_compatible(const char *self, const char *remote)
> +{
> + if (strlen(remote) != strlen(self))
> + return false;
> +
> + return (strncmp(self, remote, strlen(self))) ? false : true;
> +}
> +
> +ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private 
> *dev_priv)
> +{
> + if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
> + return -ENODEV;
> +
> + 

Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image

[Xiang Zheng]

On 2019/5/5 23:37, Peter Maydell wrote:
> On Sun, 5 May 2019 at 08:02, Xiang Zheng  wrote:
>>
>> Currently we fill the memory space with two 64MB NOR images when
>> using persistent UEFI variables on virt board. Actually we only use
>> a very small(non-zero) part of the memory while the rest significant
>> large(zero) part of memory is wasted.
>>
>> So this patch checks the block status and only writes the non-zero part
>> into memory. This requires pflash devices to use sparse files for
>> backends.
> 
> Do you mean "pflash devices will no longer work if the file
> that is backing them is not sparse", or just "if the file that
> is backing them is not sparse then you won't get the benefit
> of using less memory" ?
> 

I mean the latter, if the file is not sparse, nothing would change.
I will improve this commit message in the next version.

-- 

Thanks,
Xiang

[Qemu-devel] [PATCH v2 2/2] drm/i915/gvt: export mdev device version to sysfs for Intel vGPU

[Yan Zhao]

This feature implements the version attribute for Intel's vGPU mdev
devices.

version attribute is rw.
It's used to check device compatibility for two mdev devices.
version string format and length are private for vendor driver. vendor
driver is able to define them freely.

For Intel vGPU of gen8 and gen9, the mdev device version
consists of 3 fields: "vendor id" + "device id" + "mdev type".

Reading from a vGPU's version attribute, a string is returned in below
format: --. e.g.
8086-193b-i915-GVTg_V5_2.

Writing a string to a vGPU's version attribute will trigger GVT to check
whether a vGPU identified by the written string is compatible with
current vGPU owning this version attribute. errno is returned if the two
vGPUs are incompatible. The length of written string is returned in
compatible case.

For other platforms, and for GVT not supporting vGPU live migration
feature, errnos are returned when read/write of mdev devices' version
attributes.

For old GVT versions where no version attributes exposed in sysfs, it is
regarded as not supporting vGPU live migration.

For future platforms, besides the current 2 fields in vendor proprietary
part, more fields may be added to identify Intel vGPU well for live
migration purpose.

v2:
1. removed 32 common part of version string
(Alex Williamson)
2. do not register version attribute for GVT not supporting live
migration.(Cornelia Huck)
3. for platforms out of gen8, gen9, return -EINVAL --> -ENODEV for
incompatible. (Cornelia Huck)

Cc: Alex Williamson 
Cc: Erik Skultety 
Cc: "Dr. David Alan Gilbert" 
Cc: Cornelia Huck 
Cc: "Tian, Kevin" 
Cc: Zhenyu Wang 
Cc: "Wang, Zhi A" 
c: Neo Jia 
Cc: Kirti Wankhede 

Signed-off-by: Yan Zhao 
---
 drivers/gpu/drm/i915/gvt/Makefile |  2 +-
 drivers/gpu/drm/i915/gvt/device_version.c | 87 +++
 drivers/gpu/drm/i915/gvt/gvt.c| 51 +
 drivers/gpu/drm/i915/gvt/gvt.h|  6 ++
 4 files changed, 145 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c

diff --git a/drivers/gpu/drm/i915/gvt/Makefile 
b/drivers/gpu/drm/i915/gvt/Makefile
index 271fb46d4dd0..54e209a23899 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -3,7 +3,7 @@ GVT_DIR := gvt
 GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
interrupt.o gtt.o cfg_space.o opregion.o mmio.o display.o edid.o \
execlist.o scheduler.o sched_policy.o mmio_context.o cmd_parser.o 
debugfs.o \
-   fb_decoder.o dmabuf.o page_track.o
+   fb_decoder.o dmabuf.o page_track.o device_version.o
 
 ccflags-y  += -I$(src) -I$(src)/$(GVT_DIR)
 i915-y += $(addprefix $(GVT_DIR)/, 
$(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/device_version.c 
b/drivers/gpu/drm/i915/gvt/device_version.c
new file mode 100644
index ..bd4cdcbdba95
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/device_version.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright(c) 2011-2017 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN 
THE
+ * SOFTWARE.
+ *
+ * Authors:
+ *Yan Zhao 
+ */
+#include 
+#include "i915_drv.h"
+
+static bool is_compatible(const char *self, const char *remote)
+{
+   if (strlen(remote) != strlen(self))
+   return false;
+
+   return (strncmp(self, remote, strlen(self))) ? false : true;
+}
+
+ssize_t intel_gvt_get_vfio_device_version_len(struct drm_i915_private 
*dev_priv)
+{
+   if (!IS_GEN(dev_priv, 8) && !IS_GEN(dev_priv, 9))
+   return -ENODEV;
+
+   return PAGE_SIZE;
+}
+
+ssize_t intel_gvt_get_vfio_device_version(struct drm_i915_private *dev_priv,
+   char *buf, const char *mdev_type)
+{
+   int cnt = 0, ret = 0;
+   const char *str = NULL;
+
+   /* currently only gen8 & gen9 are supported */
+   if 

[Qemu-devel] [PATCH v2 1/2] vfio/mdev: add version attribute for mdev device

[Yan Zhao]

version attribute is used to check two mdev devices' compatibility.

The key point of this version attribute is that it's rw.
User space has no need to understand internal of device version and no
need to compare versions by itself.
Compared to reading version strings from both two mdev devices being
checked, user space only reads from one mdev device's version attribute.
After getting its version string, user space writes this string into the
other mdev device's version attribute. Vendor driver of mdev device
whose version attribute being written will check device compatibility of
the two mdev devices for user space and return success for compatibility
or errno for incompatibility.
So two readings of version attributes + checking in user space are now
changed to one reading + one writing of version attributes + checking in
vendor driver.
Format and length of version strings are now private to vendor driver
who can define them freely.

 __ user space
  /\  \
 / \write
/ read  \
 __/__   ___\|/___
| version | | version |-->check compatibility
--- ---
mdev device A   mdev device B

This version attribute is optional. If a mdev device does not provide
with a version attribute, this mdev device is incompatible to all other
mdev devices.

Live migration is able to take advantage of this version attribute.
Before user space actually starts live migration, it can first check
whether two mdev devices are compatible.

v2:
1. added detailed intent and usage
2. made definition of version string completely private to vendor driver
   (Alex Williamson)
3. abandoned changes to sample mdev drivers (Alex Williamson)
4. mandatory --> optional (Cornelia Huck)
5. added description for errno (Cornelia Huck)

Cc: Alex Williamson 
Cc: Erik Skultety 
Cc: "Dr. David Alan Gilbert" 
Cc: Cornelia Huck 
Cc: "Tian, Kevin" 
Cc: Zhenyu Wang 
Cc: "Wang, Zhi A" 
Cc: Neo Jia 
Cc: Kirti Wankhede 
Cc: Daniel P. Berrangé 
Cc: Christophe de Dinechin 

Signed-off-by: Yan Zhao 
---
 Documentation/vfio-mediated-device.txt | 140 +
 1 file changed, 140 insertions(+)

diff --git a/Documentation/vfio-mediated-device.txt 
b/Documentation/vfio-mediated-device.txt
index c3f69bcaf96e..013a764968eb 100644
--- a/Documentation/vfio-mediated-device.txt
+++ b/Documentation/vfio-mediated-device.txt
@@ -202,6 +202,7 @@ Directories and files under the sysfs for Each Physical 
Device
   | |   |--- available_instances
   | |   |--- device_api
   | |   |--- description
+  | |   |--- version
   | |   |--- [devices]
   | |--- []
   | |   |--- create
@@ -209,6 +210,7 @@ Directories and files under the sysfs for Each Physical 
Device
   | |   |--- available_instances
   | |   |--- device_api
   | |   |--- description
+  | |   |--- version
   | |   |--- [devices]
   | |--- []
   |  |--- create
@@ -216,6 +218,7 @@ Directories and files under the sysfs for Each Physical 
Device
   |  |--- available_instances
   |  |--- device_api
   |  |--- description
+  |  |--- version
   |  |--- [devices]
 
 * [mdev_supported_types]
@@ -246,6 +249,143 @@ Directories and files under the sysfs for Each Physical 
Device
   This attribute should show the number of devices of type  that can 
be
   created.
 
+* version
+
+  This attribute is rw, and is optional.
+  It is used to check device compatibility between two mdev devices and is
+  accessed in pairs between the two mdev devices being checked.
+  The intent of this attribute is to make an mdev device's version opaque to
+  user space, so instead of reading two mdev devices' version strings and
+  comparing in userspace, user space should only read one mdev device's version
+  attribute, and writes this version string into the other mdev device's 
version
+  attribute. Then vendor driver of mdev device whose version attribute being
+  written would check the incoming version string and tell user space whether
+  the two mdev devices are compatible via return value. That's why this
+  attribute is writable.
+
+  when reading this attribute, it should show device version string of
+  the device of type .
+
+  This string is private to vendor driver itself. Vendor driver is able to
+  freely define format and length of device version string.
+  e.g. It can use a combination of pciid of parent device + mdev type.
+
+  When writing a string to this attribute, vendor driver should analyze this
+  string and check whether the mdev device being identified by this string is
+  compatible with the mdev device for this attribute. vendor driver should then
+  return written string's length if it regards the two mdev devices are
+  compatible; vendor driver should return negative errno if it regards the two
+  mdev devices are not compatible.
+
+  User space should treat ANY of below 

[Qemu-devel] [PATCH v2 0/2] introduction of version attribute for VFIO live migration

[Yan Zhao]

This patchset introduces a version attribute under sysfs of VFIO Mediated
devices.

This version attribute is used to check whether two mdev devices are
compatible.
user space software can take advantage of this version attribute to
determine whether to launch live migration between two mdev devices.

Patch 1 defines version attribute in Documentation/vfio-mediated-device.txt

Patch 2 uses GVT as an example to show how to expose version attribute and
check device compatibility in vendor driver.


v2:
1. renamed patched 1
2. made definition of device version string completely private to vendor
   driver
3. abandoned changes to sample mdev drivers
4. described intent and usage of version attribute more clearly.

Yan Zhao (2):
  vfio/mdev: add version attribute for mdev device
  drm/i915/gvt: export mdev device version to sysfs for Intel vGPU

 Documentation/vfio-mediated-device.txt| 135 ++
 drivers/gpu/drm/i915/gvt/Makefile |   2 +-
 drivers/gpu/drm/i915/gvt/device_version.c |  84 ++
 drivers/gpu/drm/i915/gvt/gvt.c|  51 
 drivers/gpu/drm/i915/gvt/gvt.h|   6 +
 5 files changed, 277 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/device_version.c

-- 
2.17.1

[Qemu-devel] [QEMU-PPC] [PATCH] target/ppc: Add ibm, purr and ibm, spurr device-tree properties

[Suraj Jitindar Singh]

The ibm,purr and ibm,spurr device tree properties are used to indicate
that the processor implements the Processor Utilisation of Resources
Register (PURR) and Scaled Processor Utilisation of Resources Registers
(SPURR), respectively. Each property has a single value which represents
the level of architecture supported. A value of 1 for ibm,purr means
support for the version of the PURR defined in book 3 in version 2.02 of
the architecture. A value of 1 for ibm,spurr means support for the
version of the SPURR defined in version 2.05 of the architecture.

Add these properties for all processors for which the PURR and SPURR
registers are generated.

Signed-off-by: Suraj Jitindar Singh 
---
 hw/ppc/spapr.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2ef3ce4362..8580a8dc67 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -500,7 +500,10 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, 
int offset,
 _FDT((fdt_setprop(fdt, offset, "64-bit", NULL, 0)));
 
 if (env->spr_cb[SPR_PURR].oea_read) {
-_FDT((fdt_setprop(fdt, offset, "ibm,purr", NULL, 0)));
+_FDT((fdt_setprop_cell(fdt, offset, "ibm,purr", 1)));
+}
+if (env->spr_cb[SPR_SPURR].oea_read) {
+_FDT((fdt_setprop_cell(fdt, offset, "ibm,spurr", 1)));
 }
 
 if (ppc_hash64_has(cpu, PPC_HASH64_1TSEG)) {
-- 
2.13.6

Re: [Qemu-devel] [PATCH 5/5] hw/block/pflash_cfi02: Add the DeviceReset() handler

[Wei Yang]

On Sun, May 05, 2019 at 10:06:02PM +0200, Philippe Mathieu-Daudé wrote:
>The pflash device is a child of TYPE_DEVICE, so it can implement
>the DeviceReset handler. Actually it has to implement it, else
>on machine reset it might stay in an incoherent state, as it has
>been reported in the buglink listed below.
>
>Add the DeviceReset handler and remove its call from the realize()
>function.
>
>Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1678713
>Reported-by: Laszlo Ersek 
>Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Wei Yang 


-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 4/5] hw/block/pflash_cfi02: Extract the pflash_reset() code

[Wei Yang]

On Sun, May 05, 2019 at 10:06:01PM +0200, Philippe Mathieu-Daudé wrote:
>The reset() code is used in various places, refactor it.
>
>Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Wei Yang 

>---
> hw/block/pflash_cfi02.c | 25 +++--
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
>diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
>index f2c6201f813..f321b74433c 100644
>--- a/hw/block/pflash_cfi02.c
>+++ b/hw/block/pflash_cfi02.c
>@@ -120,6 +120,17 @@ static void pflash_register_memory(PFlashCFI02 *pfl, int 
>rom_mode)
> pfl->rom_mode = rom_mode;
> }
> 
>+static void pflash_reset(PFlashCFI02 *pfl)
>+{
>+trace_pflash_reset();
>+timer_del(>timer);
>+pfl->bypass = 0;
>+pfl->wcycle = 0;
>+pfl->cmd = 0;
>+pfl->status = 0;
>+pflash_register_memory(pfl, 1);
>+}
>+
> static void pflash_timer (void *opaque)
> {
> PFlashCFI02 *pfl = opaque;
>@@ -129,11 +140,10 @@ static void pflash_timer (void *opaque)
> pfl->status ^= 0x80;
> if (pfl->bypass) {
> pfl->wcycle = 2;
>+pfl->cmd = 0;
> } else {
>-pflash_register_memory(pfl, 1);
>-pfl->wcycle = 0;
>+pflash_reset(pfl);
> }
>-pfl->cmd = 0;
> }
> 
> static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
>@@ -481,10 +491,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
> 
> /* Reset flash */
>  reset_flash:
>-trace_pflash_reset();
>-pfl->bypass = 0;
>-pfl->wcycle = 0;
>-pfl->cmd = 0;
>+pflash_reset(pfl);
> return;
> 
>  do_bypass:
>@@ -588,9 +595,7 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
>**errp)
> sysbus_init_mmio(SYS_BUS_DEVICE(dev), >mem);
> 
> timer_init_ns(>timer, QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
>-pfl->wcycle = 0;
>-pfl->cmd = 0;
>-pfl->status = 0;
>+pflash_reset(pfl);
> /* Hardcoded CFI table (mostly from SG29 Spansion flash) */
> /* Standard "QRY" string */
> pfl->cfi_table[0x10] = 'Q';
>-- 
>2.20.1

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 3/5] hw/block/pflash_cfi01: Add the DeviceReset() handler

[Wei Yang]

On Sun, May 05, 2019 at 10:06:00PM +0200, Philippe Mathieu-Daudé wrote:
>The pflash device is a child of TYPE_DEVICE, so it can implement
>the DeviceReset handler. Actually it has to implement it, else
>on machine reset it might stay in an incoherent state, as it has
>been reported in the buglink listed below.
>
>Add the DeviceReset handler and remove its call from the realize()
>function.
>
>Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1678713
>Reported-by: Laszlo Ersek 
>Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Wei Yang 


-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 2/5] hw/block/pflash_cfi01: Extract the pflash_reset() code

[Wei Yang]

On Sun, May 05, 2019 at 10:05:59PM +0200, Philippe Mathieu-Daudé wrote:
>The reset() code is used in various places, refactor it.
>
>Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Wei Yang 

>---
> hw/block/pflash_cfi01.c | 21 -
> 1 file changed, 12 insertions(+), 9 deletions(-)
>
>diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
>index 6dc04f156a7..073cd14978f 100644
>--- a/hw/block/pflash_cfi01.c
>+++ b/hw/block/pflash_cfi01.c
>@@ -108,6 +108,15 @@ static const VMStateDescription vmstate_pflash = {
> }
> };
> 
>+static void pflash_reset(PFlashCFI01 *pfl)
>+{
>+trace_pflash_reset();
>+pfl->wcycle = 0;
>+pfl->cmd = 0;
>+pfl->status = 0;
>+memory_region_rom_device_set_romd(>mem, true);
>+}
>+
> /* Perform a CFI query based on the bank width of the flash.
>  * If this code is called we know we have a device_width set for
>  * this flash.
>@@ -275,8 +284,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr 
>offset,
> default:
> /* This should never happen : reset state & treat it as a read */
> DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
>-pfl->wcycle = 0;
>-pfl->cmd = 0;
>+pflash_reset(pfl);
> /* fall through to read code */
> case 0x00:
> /* Flash area read */
>@@ -639,10 +647,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>   "\n", __func__, offset, pfl->wcycle, pfl->cmd, value);
> 
>  reset_flash:
>-trace_pflash_reset();
>-memory_region_rom_device_set_romd(>mem, true);
>-pfl->wcycle = 0;
>-pfl->cmd = 0;
>+pflash_reset(pfl);
> }
> 
> 
>@@ -757,9 +762,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
>**errp)
> pfl->max_device_width = pfl->device_width;
> }
> 
>-pfl->wcycle = 0;
>-pfl->cmd = 0;
>-pfl->status = 0;
>+pflash_reset(pfl);
> /* Hardcoded CFI table */
> /* Standard "QRY" string */
> pfl->cfi_table[0x10] = 'Q';
>-- 
>2.20.1

-- 
Wei Yang
Help you, Help me

Re: [Qemu-devel] [PATCH 1/5] hw/block/pflash_cfi01: Removed an unused timer

[Wei Yang]

On Sun, May 05, 2019 at 10:05:58PM +0200, Philippe Mathieu-Daudé wrote:
>The 'CFI01' NOR flash was introduced in commit 29133e9a0fff, with
>timing modelled. One year later, the CFI02 model was introduced
>(commit 05ee37ebf630) based on the CFI01 model. As noted in the
>header, "It does not support timings". 12 years later, we never
>had to model the device timings. Time to remove the unused timer,
>we can still add it back if required.
>
>Suggested-by: Laszlo Ersek 
>Signed-off-by: Philippe Mathieu-Daudé 

Reviewed-by: Wei Yang 

>---
>Yes, I plan to model those timings later. Actually I have a series
>working, but I'd rather first
>1/ refactor common code between the both CFI implementations,
>2/ discuss on list whether or not use timings for the Virt flash.
>---
> hw/block/pflash_cfi01.c | 15 ---
> 1 file changed, 15 deletions(-)
>
>diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
>index 16dfae14b80..6dc04f156a7 100644
>--- a/hw/block/pflash_cfi01.c
>+++ b/hw/block/pflash_cfi01.c
>@@ -42,7 +42,6 @@
> #include "hw/block/flash.h"
> #include "sysemu/block-backend.h"
> #include "qapi/error.h"
>-#include "qemu/timer.h"
> #include "qemu/bitops.h"
> #include "qemu/host-utils.h"
> #include "qemu/log.h"
>@@ -86,7 +85,6 @@ struct PFlashCFI01 {
> uint8_t cfi_table[0x52];
> uint64_t counter;
> unsigned int writeblock_size;
>-QEMUTimer *timer;
> MemoryRegion mem;
> char *name;
> void *storage;
>@@ -110,18 +108,6 @@ static const VMStateDescription vmstate_pflash = {
> }
> };
> 
>-static void pflash_timer (void *opaque)
>-{
>-PFlashCFI01 *pfl = opaque;
>-
>-trace_pflash_timer_expired(pfl->cmd);
>-/* Reset flash */
>-pfl->status ^= 0x80;
>-memory_region_rom_device_set_romd(>mem, true);
>-pfl->wcycle = 0;
>-pfl->cmd = 0;
>-}
>-
> /* Perform a CFI query based on the bank width of the flash.
>  * If this code is called we know we have a device_width set for
>  * this flash.
>@@ -771,7 +757,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
>**errp)
> pfl->max_device_width = pfl->device_width;
> }
> 
>-pfl->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
> pfl->wcycle = 0;
> pfl->cmd = 0;
> pfl->status = 0;
>-- 
>2.20.1

-- 
Wei Yang
Help you, Help me

[Qemu-devel] [PATCH] hw/sd/sdcard: Use the available enums

[Philippe Mathieu-Daudé]

We already define SDCardModes/SDCardStates as enums. Declare
the mode/state as enums too, this make gdb debugging sessions
friendlier: instead of numbers, the mode/state name is displayed.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sd/sd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index aaab15f3868..a66b3d5b45e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -96,8 +96,8 @@ struct SDState {
 BlockBackend *blk;
 bool spi;
 
-uint32_t mode;/* current card mode, one of SDCardModes */
-int32_t state;/* current card state, one of SDCardStates */
+enum SDCardModes mode;
+enum SDCardStates state;
 uint32_t vhs;
 bool wp_switch;
 unsigned long *wp_groups;
@@ -1640,7 +1640,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest 
*req)
 
 int sd_do_command(SDState *sd, SDRequest *req,
   uint8_t *response) {
-int last_state;
+enum SDCardStates last_state;
 sd_rsp_type_t rtype;
 int rsplen;
 
-- 
2.20.1

[Qemu-devel] [PATCH] hw/display/cirrus_vga: Remove unused include

[Philippe Mathieu-Daudé]

Commit ce3cf70edaaf split the ISA device out of the PCI one,
but forgot to remove the "hw/loader.h" header inclusion (the ISA
device calls rom_add_vga()).  Remove the now unused include.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/display/cirrus_vga.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index a0e71469f4d..f67df3c1f6f 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -33,7 +33,6 @@
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
 #include "ui/pixel_ops.h"
-#include "hw/loader.h"
 #include "cirrus_vga_internal.h"
 
 /*
-- 
2.20.1

[Qemu-devel] [PATCH 1/1] MAINTAINERS: Add an entry for the Parallel NOR Flash devices

[Philippe Mathieu-Daudé]

Step in to maintain it, since I have some familiarity with
the technology.

Signed-off-by: Philippe Mathieu-Daudé 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 66ddbda9c95..633f6315536 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1391,6 +1391,13 @@ F: include/hw/net/
 F: tests/virtio-net-test.c
 T: git https://github.com/jasowang/qemu.git net
 
+Parallel NOR Flash devices
+M: Philippe Mathieu-Daudé 
+T: git https://gitlab.com/philmd/qemu.git pflash-next
+S: Maintained
+F: hw/block/pflash_cfi*.c
+F: include/hw/block/flash.h
+
 SCSI
 M: Paolo Bonzini 
 R: Fam Zheng 
-- 
2.20.1

[Qemu-devel] [PATCH 0/1] MAINTAINERS: Step in as maintainer for the parallel NOR flash devices

[Philippe Mathieu-Daudé]

The parallel NOR flash models don't have a specific maintainer and
default to the 'Block layer core' section.
Step in to maintain them.
The section still get covered by the Block layer team, but the idea
is to offload them.

The two devices are very similar (same technology), the difference
is mostly the protocol to access them.

Amusingly, between the two devices, the 'CFI01' which is used in
enterprise grade products on ARM/X86 archs is the one that received
the less care, while the 'CFI02' used by hobbyist boards is the
more reliable.

To some extent I plan to re-unify the models, and improve testing.

I'm looking for co-maintainers or designated reviewers.
(I asked Stephen Checkoway for help but unfortunately he can't).

Any volunteer?

Regards,

Phil.

PD: I Cc'ed all the people who made sinificant modification in the
devices the last few years.

Philippe Mathieu-Daudé (1):
  MAINTAINERS: Add an entry for the Parallel NOR Flash devices

 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

-- 
2.20.1

[Qemu-devel] [PATCH 12/13] hw/block/pflash_cfi02: Fix command address comparison

[Philippe Mathieu-Daudé]

From: Stephen Checkoway 

Most AMD commands only examine 11 bits of the address. This masks the
addresses used in the comparison to 11 bits. The exceptions are word or
sector addresses which use offset directly rather than the shifted
offset, boff.

Signed-off-by: Stephen Checkoway 
Acked-by: Thomas Huth 
Message-Id: <20190426162624.55977-4-stephen.checko...@oberlin.edu>
[PMD: Prepend 'hw/' in patch subject]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c   |  8 +++-
 tests/pflash-cfi02-test.c | 12 ++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 59daade2ee6..4c17dbf99f4 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -297,11 +297,13 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 
 DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
 offset, value, width);
-boff = offset & (pfl->sector_len - 1);
+boff = offset;
 if (pfl->width == 2)
 boff = boff >> 1;
 else if (pfl->width == 4)
 boff = boff >> 2;
+/* Only the least-significant 11 bits are used in most cases. */
+boff &= 0x7FF;
 switch (pfl->wcycle) {
 case 0:
 /* Set the device in I/O access mode if required */
@@ -520,6 +522,10 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
+/* Only 11 bits are used in the comparison. */
+pfl->unlock_addr0 &= 0x7FF;
+pfl->unlock_addr1 &= 0x7FF;
+
 chip_len = pfl->sector_len * pfl->nb_blocs;
 
 memory_region_init_rom_device(>orig_mem, OBJECT(pfl),
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 3c37465499a..1e5d9124b71 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -22,8 +22,8 @@
 
 #define FLASH_WIDTH 2
 #define CFI_ADDR (FLASH_WIDTH * 0x55)
-#define UNLOCK0_ADDR (FLASH_WIDTH * 0x)
-#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+#define UNLOCK0_ADDR (FLASH_WIDTH * 0x555)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AA)
 
 #define CFI_CMD 0x98
 #define UNLOCK0_CMD 0xAA
@@ -190,6 +190,14 @@ static void test_flash(const void *opaque)
 g_assert_cmpint(flash_read(6), ==, 0xCDEF);
 g_assert_cmpint(flash_read(8), ==, 0x);
 
+/* Test ignored high order bits of address. */
+flash_write(FLASH_WIDTH * 0x, UNLOCK0_CMD);
+flash_write(FLASH_WIDTH * 0x2AAA, UNLOCK1_CMD);
+flash_write(FLASH_WIDTH * 0x, AUTOSELECT_CMD);
+g_assert_cmpint(flash_read(FLASH_WIDTH * 0x), ==, 0x00BF);
+g_assert_cmpint(flash_read(FLASH_WIDTH * 0x0001), ==, 0x236D);
+reset();
+
 qtest_quit(global_qtest);
 }
 
-- 
2.20.1

[Qemu-devel] [PATCH 07/13] hw/block/pflash_cfi02: Simplify a statement using fall through

[Philippe Mathieu-Daudé]

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-3-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 9673eee969f..1e006e7bf3a 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -241,10 +241,10 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr 
offset,
 case 0x0E:
 case 0x0F:
 ret = boff & 0x01 ? pfl->ident3 : pfl->ident2;
-if (ret == (uint8_t)-1) {
-goto flash_read;
+if (ret != (uint8_t)-1) {
+break;
 }
-break;
+/* Fall through to data read. */
 default:
 goto flash_read;
 }
-- 
2.20.1

[Qemu-devel] [PATCH 13/13] hw/block/pflash_cfi02: Use the chip erase time specified in the CFI table

[Philippe Mathieu-Daudé]

From: Stephen Checkoway 

When erasing the chip, use the typical time specified in the CFI table
rather than arbitrarily selecting 5 seconds.

Since the currently unconfigurable value set in the table is 12, this
means a chip erase takes 4096 ms so this isn't a big change in behavior.

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-11-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Prepend 'hw/' in patch subject]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 4c17dbf99f4..49cd9ed0f91 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -429,9 +429,9 @@ static void pflash_write(void *opaque, hwaddr offset, 
uint64_t value,
 pflash_update(pfl, 0, pfl->chip_len);
 }
 set_dq7(pfl, 0x00);
-/* Let's wait 5 seconds before chip erase is done */
+/* Wait the time specified at CFI address 0x22. */
 timer_mod(>timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-  (NANOSECONDS_PER_SECOND * 5));
+  (1ULL << pfl->cfi_table[0x22]) * SCALE_MS);
 break;
 case 0x30:
 /* Sector erase */
-- 
2.20.1

[Qemu-devel] [PATCH 03/13] tests/pflash-cfi02: Use IEC binary prefixes for size constants

[Philippe Mathieu-Daudé]

Using IEC binary prefixes in order to make the code more readable.

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/pflash-cfi02-test.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index ff775618c02..3c37465499a 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -8,6 +8,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "libqtest.h"
 
 /*
@@ -16,7 +17,7 @@
  * all. In particular, we're limited to a 16-bit wide flash device.
  */
 
-#define MP_FLASH_SIZE_MAX (32 * 1024 * 1024)
+#define MP_FLASH_SIZE_MAX (32 * MiB)
 #define BASE_ADDR (0x1ULL - MP_FLASH_SIZE_MAX)
 
 #define FLASH_WIDTH 2
@@ -205,7 +206,7 @@ int main(int argc, char **argv)
 char *image_path;
 int fd = g_file_open_tmp("pflash-cfi02-XX.raw", _path, );
 g_assert_no_error(error);
-if (ftruncate(fd, 8 * 1024 * 1024) < 0) {
+if (ftruncate(fd, 8 * MiB) < 0) {
 g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
strerror(errno));
 close(fd);
-- 
2.20.1

[Qemu-devel] [PATCH 01/13] tests/pflash-cfi02: Add test for supported CFI commands

[Philippe Mathieu-Daudé]

From: Stephen Checkoway 

Test the AMD command set for parallel flash chips. This test uses an
ARM musicpal board with a pflash drive to test the following list of
currently-supported commands.
- Autoselect
- CFI
- Sector erase
- Chip erase
- Program
- Unlock bypass
- Reset

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-2-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: reworded the patch subject]
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/Makefile.include|   2 +
 tests/pflash-cfi02-test.c | 225 ++
 2 files changed, 227 insertions(+)
 create mode 100644 tests/pflash-cfi02-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 7c8b9c84b24..b84a308d9a1 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -263,6 +263,7 @@ check-qtest-arm-y += tests/m25p80-test$(EXESUF)
 check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
 check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
 check-qtest-arm-y += tests/hexloader-test$(EXESUF)
+check-qtest-arm-$(CONFIG_PFLASH_CFI02) += tests/pflash-cfi02-test$(EXESUF)
 
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
@@ -773,6 +774,7 @@ tests/device-introspect-test$(EXESUF): 
tests/device-introspect-test.o
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
 tests/hexloader-test$(EXESUF): tests/hexloader-test.o
+tests/pflash-cfi02$(EXESUF): tests/pflash-cfi02-test.o
 tests/endianness-test$(EXESUF): tests/endianness-test.o
 tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
 tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-spapr-obj-y)
diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
new file mode 100644
index 000..40af1bb523e
--- /dev/null
+++ b/tests/pflash-cfi02-test.c
@@ -0,0 +1,225 @@
+/*
+ * QTest testcase for parallel flash with AMD command set
+ *
+ * Copyright (c) 2019 Stephen Checkoway
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+/*
+ * To test the pflash_cfi02 device, we run QEMU with the musicpal machine with
+ * a pflash drive. This enables us to test some flash configurations, but not
+ * all. In particular, we're limited to a 16-bit wide flash device.
+ */
+
+#define MP_FLASH_SIZE_MAX (32 * 1024 * 1024)
+#define BASE_ADDR (0x1ULL - MP_FLASH_SIZE_MAX)
+
+#define FLASH_WIDTH 2
+#define CFI_ADDR (FLASH_WIDTH * 0x55)
+#define UNLOCK0_ADDR (FLASH_WIDTH * 0x)
+#define UNLOCK1_ADDR (FLASH_WIDTH * 0x2AAA)
+
+#define CFI_CMD 0x98
+#define UNLOCK0_CMD 0xAA
+#define UNLOCK1_CMD 0x55
+#define AUTOSELECT_CMD 0x90
+#define RESET_CMD 0xF0
+#define PROGRAM_CMD 0xA0
+#define SECTOR_ERASE_CMD 0x30
+#define CHIP_ERASE_CMD 0x10
+#define UNLOCK_BYPASS_CMD 0x20
+#define UNLOCK_BYPASS_RESET_CMD 0x00
+
+static char image_path[] = "/tmp/qtest.XX";
+
+static inline void flash_write(uint64_t byte_addr, uint16_t data)
+{
+qtest_writew(global_qtest, BASE_ADDR + byte_addr, data);
+}
+
+static inline uint16_t flash_read(uint64_t byte_addr)
+{
+return qtest_readw(global_qtest, BASE_ADDR + byte_addr);
+}
+
+static void unlock(void)
+{
+flash_write(UNLOCK0_ADDR, UNLOCK0_CMD);
+flash_write(UNLOCK1_ADDR, UNLOCK1_CMD);
+}
+
+static void reset(void)
+{
+flash_write(0, RESET_CMD);
+}
+
+static void sector_erase(uint64_t byte_addr)
+{
+unlock();
+flash_write(UNLOCK0_ADDR, 0x80);
+unlock();
+flash_write(byte_addr, SECTOR_ERASE_CMD);
+}
+
+static void wait_for_completion(uint64_t byte_addr)
+{
+/* If DQ6 is toggling, step the clock and ensure the toggle stops. */
+if ((flash_read(byte_addr) & 0x40) ^ (flash_read(byte_addr) & 0x40)) {
+/* Wait for erase or program to finish. */
+clock_step_next();
+/* Ensure that DQ6 has stopped toggling. */
+g_assert_cmpint(flash_read(byte_addr), ==, flash_read(byte_addr));
+}
+}
+
+static void bypass_program(uint64_t byte_addr, uint16_t data)
+{
+flash_write(UNLOCK0_ADDR, PROGRAM_CMD);
+flash_write(byte_addr, data);
+/*
+ * Data isn't valid until DQ6 stops toggling. We don't model this as
+ * writes are immediate, but if this changes in the future, we can wait
+ * until the program is complete.
+ */
+wait_for_completion(byte_addr);
+}
+
+static void program(uint64_t byte_addr, uint16_t data)
+{
+unlock();
+bypass_program(byte_addr, data);
+}
+
+static void chip_erase(void)
+{
+unlock();
+flash_write(UNLOCK0_ADDR, 0x80);
+unlock();
+flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD);
+}
+
+static void test_flash(void)
+{
+global_qtest = qtest_initf("-M musicpal,accel=qtest "
+   "-drive 
if=pflash,file=%s,format=raw,copy-on-read",
+   

[Qemu-devel] [PATCH 09/13] hw/block/pflash_cfi02: Use the ldst API in pflash_read()

[Philippe Mathieu-Daudé]

The load/store API eases code review.

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-3-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 26 ++
 1 file changed, 6 insertions(+), 20 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 69e0086324e..2777167af11 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -196,34 +196,20 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr 
offset,
 case 0x00:
 flash_read:
 /* Flash area read */
-p = pfl->storage;
+p = (uint8_t *)pfl->storage + offset;
+if (pfl->be) {
+ret = ldn_be_p(p, width);
+} else {
+ret = ldn_le_p(p, width);
+}
 switch (width) {
 case 1:
-ret = p[offset];
 trace_pflash_data_read8(offset, ret);
 break;
 case 2:
-if (be) {
-ret = p[offset] << 8;
-ret |= p[offset + 1];
-} else {
-ret = p[offset];
-ret |= p[offset + 1] << 8;
-}
 trace_pflash_data_read16(offset, ret);
 break;
 case 4:
-if (be) {
-ret = p[offset] << 24;
-ret |= p[offset + 1] << 16;
-ret |= p[offset + 2] << 8;
-ret |= p[offset + 3];
-} else {
-ret = p[offset];
-ret |= p[offset + 1] << 8;
-ret |= p[offset + 2] << 16;
-ret |= p[offset + 3] << 24;
-}
 trace_pflash_data_read32(offset, ret);
 break;
 }
-- 
2.20.1

[Qemu-devel] [PATCH 06/13] hw/block/pflash_cfi02: Add helpers to manipulate the status bits

[Philippe Mathieu-Daudé]

Pull out all of the code to modify the status into simple helper
functions. Status handling becomes more complex once multiple
chips are interleaved to produce a single device.

No change in functionality is intended with this commit.

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-3-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 39 ++-
 1 file changed, 34 insertions(+), 5 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index b27796d74d2..9673eee969f 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -100,6 +100,31 @@ struct PFlashCFI02 {
 void *storage;
 };
 
+/*
+ * Toggle status bit DQ7.
+ */
+static inline void toggle_dq7(PFlashCFI02 *pfl)
+{
+pfl->status ^= 0x80;
+}
+
+/*
+ * Set status bit DQ7 to bit 7 of value.
+ */
+static inline void set_dq7(PFlashCFI02 *pfl, uint8_t value)
+{
+pfl->status &= 0x7F;
+pfl->status |= value & 0x80;
+}
+
+/*
+ * Toggle status bit DQ6.
+ */
+static inline void toggle_dq6(PFlashCFI02 *pfl)
+{
+pfl->status ^= 0x40;
+}
+
 /*
  * Set up replicated mappings of the same region.
  */
@@ -129,7 +154,7 @@ static void pflash_timer (void *opaque)
 
 trace_pflash_timer_expired(pfl->cmd);
 /* Reset flash */
-pfl->status ^= 0x80;
+toggle_dq7(pfl);
 if (pfl->bypass) {
 pfl->wcycle = 2;
 } else {
@@ -232,7 +257,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
 ret = pfl->status;
 DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
 /* Toggle bit 6 */
-pfl->status ^= 0x40;
+toggle_dq6(pfl);
 break;
 case 0x98:
 /* CFI query mode */
@@ -381,7 +406,11 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 break;
 }
 }
-pfl->status = 0x00 | ~(value & 0x80);
+/*
+ * While programming, status bit DQ7 should hold the opposite
+ * value from how it was programmed.
+ */
+set_dq7(pfl, ~value);
 /* Let's pretend write is immediate */
 if (pfl->bypass)
 goto do_bypass;
@@ -429,7 +458,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 memset(pfl->storage, 0xFF, pfl->chip_len);
 pflash_update(pfl, 0, pfl->chip_len);
 }
-pfl->status = 0x00;
+set_dq7(pfl, 0x00);
 /* Let's wait 5 seconds before chip erase is done */
 timer_mod(>timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
   (NANOSECONDS_PER_SECOND * 5));
@@ -444,7 +473,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 memset(p + offset, 0xFF, pfl->sector_len);
 pflash_update(pfl, offset, pfl->sector_len);
 }
-pfl->status = 0x00;
+set_dq7(pfl, 0x00);
 /* Let's wait 1/2 second before sector erase is done */
 timer_mod(>timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
   (NANOSECONDS_PER_SECOND / 2));
-- 
2.20.1

[Qemu-devel] [PATCH 11/13] hw/block/pflash_cfi02: Unify the MemoryRegionOps

[Philippe Mathieu-Daudé]

The pflash_read()/pflash_write() can check the device endianess
via the pfl->be variable, so remove the 'int be' argument.

Since the big/little MemoryRegionOps are now identical, it is
pointless to declare them both. Unify them.

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-3-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Extracted from bigger patch to ease review]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 62 +++--
 1 file changed, 16 insertions(+), 46 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index adfa39f9b5e..59daade2ee6 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -186,11 +186,11 @@ static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr 
offset,
 return ret;
 }
 
-static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
-int width, int be)
+static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
 {
+PFlashCFI02 *pfl = opaque;
 hwaddr boff;
-uint32_t ret;
+uint64_t ret;
 
 ret = -1;
 trace_pflash_read(offset, pfl->cmd, width, pfl->wcycle);
@@ -238,14 +238,14 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr 
offset,
 default:
 ret = pflash_data_read(pfl, offset, width);
 }
-DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, 
ret);
+DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx64 "\n", __func__, boff, 
ret);
 break;
 case 0xA0:
 case 0x10:
 case 0x30:
 /* Status register read */
 ret = pfl->status;
-DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
+DPRINTF("%s: status %" PRIx64 "\n", __func__, ret);
 /* Toggle bit 6 */
 toggle_dq6(pfl);
 break;
@@ -263,8 +263,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
 }
 
 /* update flash content on disk */
-static void pflash_update(PFlashCFI02 *pfl, int offset,
-  int size)
+static void pflash_update(PFlashCFI02 *pfl, int offset, int size)
 {
 int offset_end;
 if (pfl->blk) {
@@ -277,9 +276,10 @@ static void pflash_update(PFlashCFI02 *pfl, int offset,
 }
 }
 
-static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
- uint32_t value, int width, int be)
+static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
+ unsigned int width)
 {
+PFlashCFI02 *pfl = opaque;
 hwaddr boff;
 uint8_t *p;
 uint8_t cmd;
@@ -295,7 +295,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 trace_pflash_write(offset, value, width, pfl->wcycle);
 offset &= pfl->chip_len - 1;
 
-DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx32 " %d\n", __func__,
+DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx64 " %d\n", __func__,
 offset, value, width);
 boff = offset & (pfl->sector_len - 1);
 if (pfl->width == 2)
@@ -492,39 +492,9 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 pfl->cmd = 0;
 }
 
-static uint64_t pflash_be_readfn(void *opaque, hwaddr addr, unsigned size)
-{
-return pflash_read(opaque, addr, size, 1);
-}
-
-static void pflash_be_writefn(void *opaque, hwaddr addr,
-  uint64_t value, unsigned size)
-{
-pflash_write(opaque, addr, value, size, 1);
-}
-
-static uint64_t pflash_le_readfn(void *opaque, hwaddr addr, unsigned size)
-{
-return pflash_read(opaque, addr, size, 0);
-}
-
-static void pflash_le_writefn(void *opaque, hwaddr addr,
-  uint64_t value, unsigned size)
-{
-pflash_write(opaque, addr, value, size, 0);
-}
-
-static const MemoryRegionOps pflash_cfi02_ops_be = {
-.read = pflash_be_readfn,
-.write = pflash_be_writefn,
-.valid.min_access_size = 1,
-.valid.max_access_size = 4,
-.endianness = DEVICE_NATIVE_ENDIAN,
-};
-
-static const MemoryRegionOps pflash_cfi02_ops_le = {
-.read = pflash_le_readfn,
-.write = pflash_le_writefn,
+static const MemoryRegionOps pflash_cfi02_ops = {
+.read = pflash_read,
+.write = pflash_write,
 .valid.min_access_size = 1,
 .valid.max_access_size = 4,
 .endianness = DEVICE_NATIVE_ENDIAN,
@@ -552,9 +522,9 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
**errp)
 
 chip_len = pfl->sector_len * pfl->nb_blocs;
 
-memory_region_init_rom_device(>orig_mem, OBJECT(pfl), pfl->be ?
-  _cfi02_ops_be : _cfi02_ops_le,
-  pfl, pfl->name, chip_len, _err);
+memory_region_init_rom_device(>orig_mem, OBJECT(pfl),
+  _cfi02_ops, pfl, pfl->name,
+  chip_len, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 return;
-- 
2.20.1

[Qemu-devel] [PATCH 04/13] hw/block/pflash_cfi02: Fix debug format string

[Philippe Mathieu-Daudé]

Always compile the debug code to prevent format string to bitrot.

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-3-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Extracted from bigger patch, use PRIx32]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 16 +++-
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f2c6201f813..f88e09cebaa 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -46,15 +46,13 @@
 #include "hw/sysbus.h"
 #include "trace.h"
 
-//#define PFLASH_DEBUG
-#ifdef PFLASH_DEBUG
+#define PFLASH_DEBUG false
 #define DPRINTF(fmt, ...)  \
 do {   \
-fprintf(stderr, "PFLASH: " fmt , ## __VA_ARGS__);   \
+if (PFLASH_DEBUG) {\
+fprintf(stderr, "PFLASH: " fmt, ## __VA_ARGS__);   \
+}  \
 } while (0)
-#else
-#define DPRINTF(fmt, ...) do { } while (0)
-#endif
 
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
@@ -220,14 +218,14 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr 
offset,
 default:
 goto flash_read;
 }
-DPRINTF("%s: ID " TARGET_FMT_plx " %x\n", __func__, boff, ret);
+DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, 
ret);
 break;
 case 0xA0:
 case 0x10:
 case 0x30:
 /* Status register read */
 ret = pfl->status;
-DPRINTF("%s: status %x\n", __func__, ret);
+DPRINTF("%s: status %" PRIx32 "\n", __func__, ret);
 /* Toggle bit 6 */
 pfl->status ^= 0x40;
 break;
@@ -277,7 +275,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 trace_pflash_write(offset, value, width, pfl->wcycle);
 offset &= pfl->chip_len - 1;
 
-DPRINTF("%s: offset " TARGET_FMT_plx " %08x %d\n", __func__,
+DPRINTF("%s: offset " TARGET_FMT_plx " %08" PRIx32 " %d\n", __func__,
 offset, value, width);
 boff = offset & (pfl->sector_len - 1);
 if (pfl->width == 2)
-- 
2.20.1

[Qemu-devel] [PATCH 08/13] hw/block/pflash_cfi02: Use the ldst API in pflash_write()

[Philippe Mathieu-Daudé]

The load/store API eases code review.

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-3-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 38 --
 1 file changed, 8 insertions(+), 30 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 1e006e7bf3a..69e0086324e 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -373,38 +373,16 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 goto check_unlock0;
 case 0xA0:
 trace_pflash_data_write(offset, value, width, 0);
-p = pfl->storage;
 if (!pfl->ro) {
-switch (width) {
-case 1:
-p[offset] &= value;
-pflash_update(pfl, offset, 1);
-break;
-case 2:
-if (be) {
-p[offset] &= value >> 8;
-p[offset + 1] &= value;
-} else {
-p[offset] &= value;
-p[offset + 1] &= value >> 8;
-}
-pflash_update(pfl, offset, 2);
-break;
-case 4:
-if (be) {
-p[offset] &= value >> 24;
-p[offset + 1] &= value >> 16;
-p[offset + 2] &= value >> 8;
-p[offset + 3] &= value;
-} else {
-p[offset] &= value;
-p[offset + 1] &= value >> 8;
-p[offset + 2] &= value >> 16;
-p[offset + 3] &= value >> 24;
-}
-pflash_update(pfl, offset, 4);
-break;
+p = (uint8_t *)pfl->storage + offset;
+if (pfl->be) {
+uint64_t current = ldn_be_p(p, width);
+stn_be_p(p, width, current & value);
+} else {
+uint64_t current = ldn_le_p(p, width);
+stn_le_p(p, width, current & value);
 }
+pflash_update(pfl, offset, width);
 }
 /*
  * While programming, status bit DQ7 should hold the opposite
-- 
2.20.1

[Qemu-devel] [PATCH 05/13] hw/block/pflash_cfi02: Add an enum to define the write cycles

[Philippe Mathieu-Daudé]

No change in functionality is intended with this commit.

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-3-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Extracted from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f88e09cebaa..b27796d74d2 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -56,6 +56,11 @@ do {   \
 
 #define PFLASH_LAZY_ROMD_THRESHOLD 42
 
+/* Special write cycles for CFI queries. */
+enum {
+WCYCLE_CFI  = 7,
+};
+
 struct PFlashCFI02 {
 /*< private >*/
 SysBusDevice parent_obj;
@@ -293,7 +298,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 if (boff == 0x55 && cmd == 0x98) {
 enter_CFI_mode:
 /* Enter CFI query mode */
-pfl->wcycle = 7;
+pfl->wcycle = WCYCLE_CFI;
 pfl->cmd = 0x98;
 return;
 }
@@ -465,7 +470,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 goto reset_flash;
 }
 break;
-case 7: /* Special value for CFI queries */
+case WCYCLE_CFI: /* Special value for CFI queries */
 DPRINTF("%s: invalid write in CFI query mode\n", __func__);
 goto reset_flash;
 default:
-- 
2.20.1

[Qemu-devel] [PATCH 10/13] hw/block/pflash_cfi02: Extract the pflash_data_read() function

[Philippe Mathieu-Daudé]

Extract the code block in a new function, remove a goto statement.

Signed-off-by: Stephen Checkoway 
Message-Id: <20190426162624.55977-3-stephen.checko...@oberlin.edu>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[PMD: Extracted from bigger patch, remove the XXX tracing comment]
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 44 ++---
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index 2777167af11..adfa39f9b5e 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -164,12 +164,33 @@ static void pflash_timer (void *opaque)
 pfl->cmd = 0;
 }
 
+/*
+ * Read data from flash.
+ */
+static uint64_t pflash_data_read(PFlashCFI02 *pfl, hwaddr offset,
+ unsigned int width)
+{
+uint8_t *p = (uint8_t *)pfl->storage + offset;
+uint64_t ret = pfl->be ? ldn_be_p(p, width) : ldn_le_p(p, width);
+switch (width) {
+case 1:
+trace_pflash_data_read8(offset, ret);
+break;
+case 2:
+trace_pflash_data_read16(offset, ret);
+break;
+case 4:
+trace_pflash_data_read32(offset, ret);
+break;
+}
+return ret;
+}
+
 static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
 int width, int be)
 {
 hwaddr boff;
 uint32_t ret;
-uint8_t *p;
 
 ret = -1;
 trace_pflash_read(offset, pfl->cmd, width, pfl->wcycle);
@@ -194,25 +215,8 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr 
offset,
 case 0x80:
 /* We accept reads during second unlock sequence... */
 case 0x00:
-flash_read:
 /* Flash area read */
-p = (uint8_t *)pfl->storage + offset;
-if (pfl->be) {
-ret = ldn_be_p(p, width);
-} else {
-ret = ldn_le_p(p, width);
-}
-switch (width) {
-case 1:
-trace_pflash_data_read8(offset, ret);
-break;
-case 2:
-trace_pflash_data_read16(offset, ret);
-break;
-case 4:
-trace_pflash_data_read32(offset, ret);
-break;
-}
+ret = pflash_data_read(pfl, offset, width);
 break;
 case 0x90:
 /* flash ID read */
@@ -232,7 +236,7 @@ static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
 }
 /* Fall through to data read. */
 default:
-goto flash_read;
+ret = pflash_data_read(pfl, offset, width);
 }
 DPRINTF("%s: ID " TARGET_FMT_plx " %" PRIx32 "\n", __func__, boff, 
ret);
 break;
-- 
2.20.1

[Qemu-devel] [PATCH 02/13] tests/pflash-cfi02: Use the GLib API

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/pflash-cfi02-test.c | 28 +---
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/tests/pflash-cfi02-test.c b/tests/pflash-cfi02-test.c
index 40af1bb523e..ff775618c02 100644
--- a/tests/pflash-cfi02-test.c
+++ b/tests/pflash-cfi02-test.c
@@ -35,8 +35,6 @@
 #define UNLOCK_BYPASS_CMD 0x20
 #define UNLOCK_BYPASS_RESET_CMD 0x00
 
-static char image_path[] = "/tmp/qtest.XX";
-
 static inline void flash_write(uint64_t byte_addr, uint16_t data)
 {
 qtest_writew(global_qtest, BASE_ADDR + byte_addr, data);
@@ -103,8 +101,9 @@ static void chip_erase(void)
 flash_write(UNLOCK0_ADDR, SECTOR_ERASE_CMD);
 }
 
-static void test_flash(void)
+static void test_flash(const void *opaque)
 {
+const char *image_path = opaque;
 global_qtest = qtest_initf("-M musicpal,accel=qtest "
"-drive 
if=pflash,file=%s,format=raw,copy-on-read",
image_path);
@@ -195,31 +194,30 @@ static void test_flash(void)
 
 static void cleanup(void *opaque)
 {
+char *image_path = opaque;
 unlink(image_path);
+g_free(image_path);
 }
 
 int main(int argc, char **argv)
 {
-int fd = mkstemp(image_path);
-if (fd == -1) {
-g_printerr("Failed to create temporary file %s: %s\n", image_path,
-   strerror(errno));
-exit(EXIT_FAILURE);
-}
+GError *error = NULL;
+char *image_path;
+int fd = g_file_open_tmp("pflash-cfi02-XX.raw", _path, );
+g_assert_no_error(error);
 if (ftruncate(fd, 8 * 1024 * 1024) < 0) {
-int error_code = errno;
+g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
+   strerror(errno));
 close(fd);
 unlink(image_path);
-g_printerr("Failed to truncate file %s to 8 MB: %s\n", image_path,
-   strerror(error_code));
 exit(EXIT_FAILURE);
 }
 close(fd);
 
-qtest_add_abrt_handler(cleanup, NULL);
+qtest_add_abrt_handler(cleanup, image_path);
 g_test_init(, , NULL);
-qtest_add_func("pflash-cfi02", test_flash);
+qtest_add_data_func("pflash-cfi02", image_path, test_flash);
 int result = g_test_run();
-cleanup(NULL);
+cleanup(image_path);
 return result;
 }
-- 
2.20.1

[Qemu-devel] [PATCH 00/13] hw/block/pflash_cfi02: Clean-up and fixes

[Philippe Mathieu-Daudé]

Hi,

While reviewing Stephen Checkoway's v4 "Implement missing AMD
pflash functionality" [*] I found it hard (for me) to digest,
so I took step by step notes. This series is the result of
those notes.
Regarding Stephen's series, this series only contains the
generic code movement and trivial cleanup. The other patches
are rather dense and I need more time to study the specs.

Stephen: If you take out the patch #2 ("Use the GLib API"),
you can rebase your series on top of this.
I'd appreciate if you can adapt your tests to use the GLib
functions, else I plan to do it later.

Regards,

Phil.

[*] https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg04595.html

Philippe Mathieu-Daudé (10):
  tests/pflash-cfi02: Use the GLib API
  tests/pflash-cfi02: Use IEC binary prefixes for size constants
  hw/block/pflash_cfi02: Fix debug format string
  hw/block/pflash_cfi02: Add an enum to define the write cycles
  hw/block/pflash_cfi02: Add helpers to manipulate the status bits
  hw/block/pflash_cfi02: Simplify a statement using fall through
  hw/block/pflash_cfi02: Use the ldst API in pflash_write()
  hw/block/pflash_cfi02: Use the ldst API in pflash_read()
  hw/block/pflash_cfi02: Extract the pflash_data_read() function
  hw/block/pflash_cfi02: Unify the MemoryRegionOps

Stephen Checkoway (3):
  tests/pflash-cfi02: Add test for supported CFI commands
  hw/block/pflash_cfi02: Fix command address comparison
  hw/block/pflash_cfi02: Use the chip erase time specified in the CFI
table

 hw/block/pflash_cfi02.c   | 234 +-
 tests/Makefile.include|   2 +
 tests/pflash-cfi02-test.c | 232 +
 3 files changed, 339 insertions(+), 129 deletions(-)
 create mode 100644 tests/pflash-cfi02-test.c

-- 
2.20.1

Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.

[Alex Bennée]

Richard Henderson  writes:

> On 5/3/19 8:27 AM, Alex Bennée wrote:
>>
>> Yoshinori Sato  writes:
>>
>>> Some RX peripheral using 8bit and 16bit registers.
>>> Added 8bit and 16bit APIs.
>>
>> Doesn't this mean the build breaks at some point? Features used by other
>> patches should be introduced first so the build remains bisectable.
>
> The only bug I would fix in the ordering is to make the change to configure
> last, so that the target/rx is not enabled while the patches are
> staging.

That will do - the key thing is each interim position in the commit log
can build on it's own.

--
Alex Bennée

Re: [Qemu-devel] [PATCH v1 8/8] target/avr: Register AVR support with the rest of QEMU, the build system, and the MAINTAINERS file

[Michael Rolnik]

Hi Richard.

I can maintain it

Sent from my cell phone, please ignore typos

On Sun, May 5, 2019, 8:57 AM Richard Henderson 
wrote:

> On 5/4/19 1:36 AM, Sarah Harris wrote:
> > Signed-off-by: Sarah Harris 
> ...
> >
> > +AVR
> > +M: Michael Rolnik 
> > +S: Odd Fixes
> > +F: target-avr/
> > +F: hw/avr/
> > +
>
> This is not how things work.  Michael wasn't up to maintaining the code 2
> years
> ago; that's why it was never committed.
>
> You would need to maintain this yourself, and for more than "Odd Fixes".
>
>
> r~
>

[Qemu-devel] [Bug 1774830] Re: qemu monitor disassembled memory dump produces incorrect output

[felix]

** Patch added: "disas.patch"
   
https://bugs.launchpad.net/qemu/+bug/1774830/+attachment/5261663/+files/disas.patch

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

Title:
  qemu monitor disassembled memory dump produces incorrect output

Status in QEMU:
  New

Bug description:
  Greetings,

  I've been using qemu-system-aarch64 to do some low-level programming
  targeting the raspberry pi 3. While I was debugging a problem in my
  code I noticed a very confusing inconsistency that I think is very
  likely a bug somewhere in how qemu's monitor produces its disassembled
  output.

  Here's my version output (installed via homebrew on macOS 10.13.4)

  $ qemu-system-aarch64 --version
  QEMU emulator version 2.12.0
  Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers

  Some system information (macOS 10.13.4):

  $ uname -a
  Darwin Lillith.local 17.5.0 Darwin Kernel Version 17.5.0: Fri Apr 13 19:32:32 
PDT 2018; root:xnu-4570.51.2~1/RELEASE_X86_64 x86_64

  Here's an example of the problem I am seeing:

  qemu-system-aarch64 -S -M raspi3 -kernel kernel8.img -monitor stdio
  QEMU 2.12.0 monitor - type 'help' for more information
  (qemu) x /32x 0x8
  0008: 0xd53800a1 0x92400421 0xb461 0xd503205f
  00080010: 0x17ff 0x58000161 0x913f 0x58000161
  00080020: 0x18e2 0x3482 0xf800843f 0x51000442
  00080030: 0x35a2 0xd2806763 0x17ff 0x
  00080040: 0x0008 0x 0x00080050 0x
  00080050: 0x 0x 0x 0x
  00080060: 0x 0x 0x 0x
  00080070: 0x 0x 0x 0x
  (qemu) x /32i 0x8
  0x0008:  d53800a1  mrs  x1, mpidr_el1
  0x00080004:  92400421  and  x1, x1, #3
  0x00080008:  b461  cbz  x1, #0x80014
  0x0008000c:  d503205f  wfe
  0x00080010:  17ff  b#0x8000c
  0x00080014:  58000161  ldr  x1, #0x80040
  0x00080018:  913f  mov  sp, x1
  0x0008001c:  58000161  ldr  x1, #0x80048
  0x00080020:  92400421  and  x1, x1, #3
  0x00080024:  b461  cbz  x1, #0x80030
  0x00080028:  d503205f  wfe
  0x0008002c:  17ff  b#0x80028
  0x00080030:  58000161  ldr  x1, #0x8005c
  0x00080034:  913f  mov  sp, x1
  0x00080038:  58000161  ldr  x1, #0x80064
  0x0008003c:  18e2  ldr  w2, #0x80058
  0x00080040:  3482  cbz  w2, #0x80050
  0x00080044:  f800843f  str  xzr, [x1], #8
  0x00080048:  51000442  sub  w2, w2, #1
  0x0008004c:  35a2  cbnz w2, #0x80040
  0x00080050:  d2806763  movz x3, #0x33b
  0x00080054:  17ff  b#0x80050
  0x00080058:    .byte0x00, 0x00, 0x00, 0x00
  0x0008005c:  0008  .byte0x00, 0x00, 0x08, 0x00
  0x00080060:    .byte0x00, 0x00, 0x00, 0x00
  0x00080064:  00080050  .byte0x50, 0x00, 0x08, 0x00
  0x00080068:    .byte0x00, 0x00, 0x00, 0x00
  0x0008006c:    .byte0x00, 0x00, 0x00, 0x00
  0x00080070:    .byte0x00, 0x00, 0x00, 0x00
  0x00080074:    .byte0x00, 0x00, 0x00, 0x00
  0x00080078:    .byte0x00, 0x00, 0x00, 0x00
  0x0008007c:    .byte0x00, 0x00, 0x00, 0x00

  Please notice that between 0x80004 thru 0x8001c is repeated for some
  reason in the /32i formatted output which also causes the addresses
  for the following bytes to also be incorrect.

  Just in order to keep things as clear as possible, I'll also attach
  the binary shown above but disassembled by objdump instead of qemu.

  $ aarch64-elf-objdump -d kernel8.elf

  kernel8.elf: file format elf64-littleaarch64

  Disassembly of section .text:

  0008 <_start>:
     8: d53800a1mrs x1, mpidr_el1
     80004: 92400421and x1, x1, #0x3
     80008: b461cbz x1, 80014 <_start+0x14>
     8000c: d503205fwfe
     80010: 17ffb   8000c <_start+0xc>
     80014: 58000161ldr x1, 80040 <_start+0x40>
     80018: 913fmov sp, x1
     8001c: 58000161ldr x1, 80048 <_start+0x48>
     80020: 18e2ldr w2, 8003c <_start+0x3c>
     80024: 3482cbz w2, 80034 <_start+0x34>
     80028: f800843fstr xzr, [x1], #8
     8002c: 51000442sub w2, w2, #0x1
     80030: 35a2cbnzw2, 80024 <_start+0x24>
     80034: d2806763mov x3, #0x33b  // #827
     80038: 17ffb   80034 <_start+0x34>
     8003c: .word   0x
     80040: 0008.word   0x0008
     80044: .word   0x
     80048: 00080050.word   0x00080050
     8004c: .word   0x

  

[Qemu-devel] [PATCH 5/5] hw/block/pflash_cfi02: Add the DeviceReset() handler

[Philippe Mathieu-Daudé]

The pflash device is a child of TYPE_DEVICE, so it can implement
the DeviceReset handler. Actually it has to implement it, else
on machine reset it might stay in an incoherent state, as it has
been reported in the buglink listed below.

Add the DeviceReset handler and remove its call from the realize()
function.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1678713
Reported-by: Laszlo Ersek 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f321b74433c..5af367d1563 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -674,6 +674,11 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
**errp)
 pfl->cfi_table[0x3c] = 0x00;
 }
 
+static void pflash_cfi02_reset(DeviceState *dev)
+{
+pflash_reset(PFLASH_CFI02(dev));
+}
+
 static Property pflash_cfi02_properties[] = {
 DEFINE_PROP_DRIVE("drive", PFlashCFI02, blk),
 DEFINE_PROP_UINT32("num-blocks", PFlashCFI02, nb_blocs, 0),
@@ -701,6 +706,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, 
void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 
+dc->reset = pflash_cfi02_reset;
 dc->realize = pflash_cfi02_realize;
 dc->unrealize = pflash_cfi02_unrealize;
 dc->props = pflash_cfi02_properties;
-- 
2.20.1

[Qemu-devel] [PATCH 4/5] hw/block/pflash_cfi02: Extract the pflash_reset() code

[Philippe Mathieu-Daudé]

The reset() code is used in various places, refactor it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi02.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
index f2c6201f813..f321b74433c 100644
--- a/hw/block/pflash_cfi02.c
+++ b/hw/block/pflash_cfi02.c
@@ -120,6 +120,17 @@ static void pflash_register_memory(PFlashCFI02 *pfl, int 
rom_mode)
 pfl->rom_mode = rom_mode;
 }
 
+static void pflash_reset(PFlashCFI02 *pfl)
+{
+trace_pflash_reset();
+timer_del(>timer);
+pfl->bypass = 0;
+pfl->wcycle = 0;
+pfl->cmd = 0;
+pfl->status = 0;
+pflash_register_memory(pfl, 1);
+}
+
 static void pflash_timer (void *opaque)
 {
 PFlashCFI02 *pfl = opaque;
@@ -129,11 +140,10 @@ static void pflash_timer (void *opaque)
 pfl->status ^= 0x80;
 if (pfl->bypass) {
 pfl->wcycle = 2;
+pfl->cmd = 0;
 } else {
-pflash_register_memory(pfl, 1);
-pfl->wcycle = 0;
+pflash_reset(pfl);
 }
-pfl->cmd = 0;
 }
 
 static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
@@ -481,10 +491,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr offset,
 
 /* Reset flash */
  reset_flash:
-trace_pflash_reset();
-pfl->bypass = 0;
-pfl->wcycle = 0;
-pfl->cmd = 0;
+pflash_reset(pfl);
 return;
 
  do_bypass:
@@ -588,9 +595,7 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
**errp)
 sysbus_init_mmio(SYS_BUS_DEVICE(dev), >mem);
 
 timer_init_ns(>timer, QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
-pfl->wcycle = 0;
-pfl->cmd = 0;
-pfl->status = 0;
+pflash_reset(pfl);
 /* Hardcoded CFI table (mostly from SG29 Spansion flash) */
 /* Standard "QRY" string */
 pfl->cfi_table[0x10] = 'Q';
-- 
2.20.1

[Qemu-devel] [PATCH 2/5] hw/block/pflash_cfi01: Extract the pflash_reset() code

[Philippe Mathieu-Daudé]

The reset() code is used in various places, refactor it.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi01.c | 21 -
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 6dc04f156a7..073cd14978f 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -108,6 +108,15 @@ static const VMStateDescription vmstate_pflash = {
 }
 };
 
+static void pflash_reset(PFlashCFI01 *pfl)
+{
+trace_pflash_reset();
+pfl->wcycle = 0;
+pfl->cmd = 0;
+pfl->status = 0;
+memory_region_rom_device_set_romd(>mem, true);
+}
+
 /* Perform a CFI query based on the bank width of the flash.
  * If this code is called we know we have a device_width set for
  * this flash.
@@ -275,8 +284,7 @@ static uint32_t pflash_read(PFlashCFI01 *pfl, hwaddr offset,
 default:
 /* This should never happen : reset state & treat it as a read */
 DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
-pfl->wcycle = 0;
-pfl->cmd = 0;
+pflash_reset(pfl);
 /* fall through to read code */
 case 0x00:
 /* Flash area read */
@@ -639,10 +647,7 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
   "\n", __func__, offset, pfl->wcycle, pfl->cmd, value);
 
  reset_flash:
-trace_pflash_reset();
-memory_region_rom_device_set_romd(>mem, true);
-pfl->wcycle = 0;
-pfl->cmd = 0;
+pflash_reset(pfl);
 }
 
 
@@ -757,9 +762,7 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
**errp)
 pfl->max_device_width = pfl->device_width;
 }
 
-pfl->wcycle = 0;
-pfl->cmd = 0;
-pfl->status = 0;
+pflash_reset(pfl);
 /* Hardcoded CFI table */
 /* Standard "QRY" string */
 pfl->cfi_table[0x10] = 'Q';
-- 
2.20.1

[Qemu-devel] [PATCH 3/5] hw/block/pflash_cfi01: Add the DeviceReset() handler

[Philippe Mathieu-Daudé]

The pflash device is a child of TYPE_DEVICE, so it can implement
the DeviceReset handler. Actually it has to implement it, else
on machine reset it might stay in an incoherent state, as it has
been reported in the buglink listed below.

Add the DeviceReset handler and remove its call from the realize()
function.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1678713
Reported-by: Laszlo Ersek 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/block/pflash_cfi01.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 073cd14978f..639b05bc4d5 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -762,7 +762,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
**errp)
 pfl->max_device_width = pfl->device_width;
 }
 
-pflash_reset(pfl);
 /* Hardcoded CFI table */
 /* Standard "QRY" string */
 pfl->cfi_table[0x10] = 'Q';
@@ -850,6 +849,11 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
**errp)
 pfl->cfi_table[0x3f] = 0x01; /* Number of protection fields */
 }
 
+static void pflash_cfi01_reset(DeviceState *dev)
+{
+pflash_reset(PFLASH_CFI01(dev));
+}
+
 static Property pflash_cfi01_properties[] = {
 DEFINE_PROP_DRIVE("drive", PFlashCFI01, blk),
 /* num-blocks is the number of blocks actually visible to the guest,
@@ -894,6 +898,7 @@ static void pflash_cfi01_class_init(ObjectClass *klass, 
void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 
+dc->reset = pflash_cfi01_reset;
 dc->realize = pflash_cfi01_realize;
 dc->props = pflash_cfi01_properties;
 dc->vmsd = _pflash;
-- 
2.20.1

[Qemu-devel] [PATCH 1/5] hw/block/pflash_cfi01: Removed an unused timer

[Philippe Mathieu-Daudé]

The 'CFI01' NOR flash was introduced in commit 29133e9a0fff, with
timing modelled. One year later, the CFI02 model was introduced
(commit 05ee37ebf630) based on the CFI01 model. As noted in the
header, "It does not support timings". 12 years later, we never
had to model the device timings. Time to remove the unused timer,
we can still add it back if required.

Suggested-by: Laszlo Ersek 
Signed-off-by: Philippe Mathieu-Daudé 
---
Yes, I plan to model those timings later. Actually I have a series
working, but I'd rather first
1/ refactor common code between the both CFI implementations,
2/ discuss on list whether or not use timings for the Virt flash.
---
 hw/block/pflash_cfi01.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index 16dfae14b80..6dc04f156a7 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -42,7 +42,6 @@
 #include "hw/block/flash.h"
 #include "sysemu/block-backend.h"
 #include "qapi/error.h"
-#include "qemu/timer.h"
 #include "qemu/bitops.h"
 #include "qemu/host-utils.h"
 #include "qemu/log.h"
@@ -86,7 +85,6 @@ struct PFlashCFI01 {
 uint8_t cfi_table[0x52];
 uint64_t counter;
 unsigned int writeblock_size;
-QEMUTimer *timer;
 MemoryRegion mem;
 char *name;
 void *storage;
@@ -110,18 +108,6 @@ static const VMStateDescription vmstate_pflash = {
 }
 };
 
-static void pflash_timer (void *opaque)
-{
-PFlashCFI01 *pfl = opaque;
-
-trace_pflash_timer_expired(pfl->cmd);
-/* Reset flash */
-pfl->status ^= 0x80;
-memory_region_rom_device_set_romd(>mem, true);
-pfl->wcycle = 0;
-pfl->cmd = 0;
-}
-
 /* Perform a CFI query based on the bank width of the flash.
  * If this code is called we know we have a device_width set for
  * this flash.
@@ -771,7 +757,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
**errp)
 pfl->max_device_width = pfl->device_width;
 }
 
-pfl->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
 pfl->wcycle = 0;
 pfl->cmd = 0;
 pfl->status = 0;
-- 
2.20.1

[Qemu-devel] [PATCH 0/5] hw/block/pflash: Add DeviceReset() handlers

[Philippe Mathieu-Daudé]

The pflash device lacks a reset() function.
When a machine is resetted, the flash might be in an
inconsistent state, leading to unexpected behavior:
https://bugzilla.redhat.com/show_bug.cgi?id=1678713

Resolve this issue by adding a DeviceReset() handler.

Both CFI01/CFI02 devices are fixed by this series.

Regards,

Phil.

Philippe Mathieu-Daudé (5):
  hw/block/pflash_cfi01: Removed an unused timer
  hw/block/pflash_cfi01: Extract the pflash_reset() code
  hw/block/pflash_cfi01: Add the DeviceReset() handler
  hw/block/pflash_cfi02: Extract the pflash_reset() code
  hw/block/pflash_cfi02: Add the DeviceReset() handler

 hw/block/pflash_cfi01.c | 31 ---
 hw/block/pflash_cfi02.c | 31 +--
 2 files changed, 33 insertions(+), 29 deletions(-)

-- 
2.20.1

Re: [Qemu-devel] [PATCH v4 10/10] block/pflash_cfi02: Use the chip erase time specified in the CFI table

[Philippe Mathieu-Daudé]

On 4/26/19 6:26 PM, Stephen Checkoway wrote:
> When erasing the chip, use the typical time specified in the CFI table
> rather than arbitrarily selecting 5 seconds.
> 
> Since the currently unconfigurable value set in the table is 12, this
> means a chip erase takes 4096 ms so this isn't a big change in behavior.
> 
> Signed-off-by: Stephen Checkoway 
> ---
>  hw/block/pflash_cfi02.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index d9087cafff..76c8af4365 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -633,9 +633,9 @@ static void pflash_write(void *opaque, hwaddr offset, 
> uint64_t value,
>  pflash_update(pfl, 0, pfl->total_len);
>  }
>  set_dq7(pfl, 0x00);
> -/* Let's wait 5 seconds before chip erase is done */
> +/* Wait the time specified at CFI address 0x22. */
>  timer_mod(>timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
> -  (NANOSECONDS_PER_SECOND * 5));
> +  (1ULL << pfl->cfi_table[0x22]) * SCALE_MS);
>  break;
>  case 0x30:
>  /* Sector erase */
> 

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH] virtfs: Add missing "id" parameter in documentation

[Greg Kurz]

Hi Thomas,

Thanks for the janitoring :)

On Sun,  5 May 2019 16:45:27 +0200
Thomas Huth  wrote:

> ... and remove the square brackets from "path" and "security_model",
> since these parameters are not optional.
> 

Well this is only true when fsdriver == local, but the other fs drivers,
ie. proxy and synth, don't need it at all. Each driver has its own set of
options actually. This should better be described with separate lines IMHO.

Also, it should be stated that "id" relates to the fs backend, ie. it
belongs to the -fsdev "id" space, not to the device that gets exposed
to the guest.

Cheers,

--
Greg

> Buglink: https://bugs.launchpad.net/qemu/+bug/1581976
> Signed-off-by: Thomas Huth 
> ---
>  qemu-options.hx | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 51802cbb26..9571ddd141 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1311,7 +1311,7 @@ DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
>  
>  STEXI
>  
> -@item -virtfs 
> @var{fsdriver}[,path=@var{path}],mount_tag=@var{mount_tag}[,security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,socket=@var{socket}|sock_fd=@var{sock_fd}][,fmode=@var{fmode}][,dmode=@var{dmode}]
> +@item -virtfs 
> @var{fsdriver},path=@var{path},mount_tag=@var{mount_tag},security_model=@var{security_model}[,id=@var{id}][,writeout=@var{writeout}][,readonly][,socket=@var{socket}|sock_fd=@var{sock_fd}][,fmode=@var{fmode}][,dmode=@var{dmode}]
>  @findex -virtfs
>  
>  The general form of a Virtual File system pass-through options are:

[Qemu-devel] [PATCH] hw/timer: Compile devices not target-dependent as common objects

[Philippe Mathieu-Daudé]

All these devices do not contain any target-specific code. While
most of them are arch-specific, they are shared between different
targets of the same arch family (ARM and AArch64, MIPS32/MIPS64,
multiple endianess, ...).
Put them into common-obj-y to compile them once for all targets.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/timer/Makefile.objs | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 0e9a4530f84..a92e22938cb 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -25,20 +25,20 @@ common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
 common-obj-$(CONFIG_XLNX_ZYNQMP) += xlnx-zynqmp-rtc.o
 common-obj-$(CONFIG_NRF51_SOC) += nrf51_timer.o
 
-obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o
-obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
-obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
-obj-$(CONFIG_EXYNOS4) += exynos4210_rtc.o
-obj-$(CONFIG_OMAP) += omap_gptimer.o
-obj-$(CONFIG_OMAP) += omap_synctimer.o
-obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
-obj-$(CONFIG_SH4) += sh_timer.o
-obj-$(CONFIG_DIGIC) += digic-timer.o
-obj-$(CONFIG_MIPS_CPS) += mips_gictimer.o
+common-obj-$(CONFIG_ALTERA_TIMER) += altera_timer.o
+common-obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
+common-obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
+common-obj-$(CONFIG_EXYNOS4) += exynos4210_rtc.o
+common-obj-$(CONFIG_OMAP) += omap_gptimer.o
+common-obj-$(CONFIG_OMAP) += omap_synctimer.o
+common-obj-$(CONFIG_PXA2XX) += pxa2xx_timer.o
+common-obj-$(CONFIG_SH4) += sh_timer.o
+common-obj-$(CONFIG_DIGIC) += digic-timer.o
+common-obj-$(CONFIG_MIPS_CPS) += mips_gictimer.o
 
 obj-$(CONFIG_MC146818RTC) += mc146818rtc.o
 
-obj-$(CONFIG_ALLWINNER_A10_PIT) += allwinner-a10-pit.o
+common-obj-$(CONFIG_ALLWINNER_A10_PIT) += allwinner-a10-pit.o
 
 common-obj-$(CONFIG_STM32F2XX_TIMER) += stm32f2xx_timer.o
 common-obj-$(CONFIG_ASPEED_SOC) += aspeed_timer.o
-- 
2.20.1

Re: [Qemu-devel] [PATCH 8/9] tcg/i386: add support for IBT

[Richard Henderson]

On 5/4/19 5:05 AM, Paolo Bonzini wrote:
> Add endbr annotations before indirect branch targets.  This lets QEMU enable
> IBT even for TCG-enabled builds.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  Makefile.target   |  2 ++
>  configure |  9 +
>  include/qemu/cpuid.h  |  5 +
>  tcg/i386/tcg-target.inc.c | 19 +++
>  4 files changed, 35 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 5/9] coroutine: add host specific coroutine backend for 64-bit s390

[Richard Henderson]

On 5/4/19 5:05 AM, Paolo Bonzini wrote:
> +  "bras %%r3, 1f\n"/* source PC will be after the BR */ \
> +  "1: aghi %%r3, 12\n" /* 4 */  \
> +  "stg %%r3, %[SCRATCH](%%r1)\n"   /* 6 save switch-back PC */  \
> +  "br %%r4\n"  /* 2 jump to destination PC */   \

Better as

larl%r3, 2f
stg %r3, SCRATCH(%r1)
br  %r4
2:


r~

Re: [Qemu-devel] [PATCH 4/9] coroutine: add host specific coroutine backend for 64-bit ARM

[Richard Henderson]

On 5/4/19 5:05 AM, Paolo Bonzini wrote:
> The speedup is similar to x86, 120 ns vs 180 ns on an APM Mustang.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  configure|  2 +-
>  scripts/qemugdb/coroutine_asm.py |  6 -
>  util/Makefile.objs   |  2 ++
>  util/coroutine-asm.c | 45 
>  4 files changed, 53 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 

> +"ldr x30, [x1, %[SCRATCH]]\n"/* load destination PC */   \
> +"ldr x1, [x1, %[SP]]\n"  /* load destination SP */   \
> +"mov sp, x1\n"   \
> +"br x30\n"   \
> +"2: \n"  \

For future reference, "bti j" (aka hint #36) goes here,
for the aarch64 branch target identification extension.


r~

Re: [Qemu-devel] [PATCH 3/9] coroutine: add host specific coroutine backend for 64-bit x86

[Richard Henderson]

On 5/4/19 5:05 AM, Paolo Bonzini wrote:
> This backend is faster (100ns vs 150ns per switch on my laptop), but
> especially it will be possible to add CET support to it.  Most of the
> code is actually not architecture specific.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  configure|  10 ++
>  scripts/qemugdb/coroutine.py |   5 +-
>  scripts/qemugdb/coroutine_asm.py |  20 +++
>  util/Makefile.objs   |   1 +
>  util/coroutine-asm.c | 230 +++
>  5 files changed, 264 insertions(+), 2 deletions(-)
>  create mode 100644 scripts/qemugdb/coroutine_asm.py
>  create mode 100644 util/coroutine-asm.c

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH RFC v8 08/12] hw/char: RX62N serical communication interface (SCI)

[Yoshinori Sato]

On Sat, 04 May 2019 00:22:44 +0900,
Alex Bennée wrote:
> 
> 
> Yoshinori Sato  writes:
> 
> 
> nit: typo in subject (serical->serial)
> 
> > This module supported only non FIFO type.
> > Hardware manual.
> > https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> >
> > Signed-off-by: Yoshinori Sato 
> > ---
> >  include/hw/char/renesas_sci.h |  45 ++
> >  hw/char/renesas_sci.c | 341 
> > ++
> >  hw/char/Kconfig   |   3 +
> >  hw/char/Makefile.objs |   2 +-
> >  4 files changed, 390 insertions(+), 1 deletion(-)
> >  create mode 100644 include/hw/char/renesas_sci.h
> >  create mode 100644 hw/char/renesas_sci.c
> >
> > diff --git a/include/hw/char/renesas_sci.h b/include/hw/char/renesas_sci.h
> > new file mode 100644
> > index 00..50d1336944
> > --- /dev/null
> > +++ b/include/hw/char/renesas_sci.h
> > @@ -0,0 +1,45 @@
> > +/*
> > + * Renesas Serial Communication Interface
> > + *
> > + * Copyright (c) 2018 Yoshinori Sato
> > + *
> > + * This code is licensed under the GPL version 2 or later.
> > + *
> > + */
> > +
> > +#include "chardev/char-fe.h"
> > +#include "qemu/timer.h"
> > +#include "hw/sysbus.h"
> > +
> > +#define TYPE_RENESAS_SCI "renesas-sci"
> > +#define RSCI(obj) OBJECT_CHECK(RSCIState, (obj), TYPE_RENESAS_SCI)
> > +
> > +enum {
> > +ERI = 0,
> > +RXI = 1,
> > +TXI = 2,
> > +TEI = 3,
> > +SCI_NR_IRQ = 4,
> > +};
> > +
> > +typedef struct {
> > +SysBusDevice parent_obj;
> > +MemoryRegion memory;
> > +
> > +uint8_t smr;
> > +uint8_t brr;
> > +uint8_t scr;
> > +uint8_t tdr;
> > +uint8_t ssr;
> > +uint8_t rdr;
> > +uint8_t scmr;
> > +uint8_t semr;
> > +
> > +uint8_t read_ssr;
> > +int64_t trtime;
> > +int64_t rx_next;
> > +QEMUTimer *timer;
> > +CharBackend chr;
> > +uint64_t input_freq;
> > +qemu_irq irq[SCI_NR_IRQ];
> > +} RSCIState;
> > diff --git a/hw/char/renesas_sci.c b/hw/char/renesas_sci.c
> > new file mode 100644
> > index 00..719fa2f938
> > --- /dev/null
> > +++ b/hw/char/renesas_sci.c
> > @@ -0,0 +1,341 @@
> > +/*
> > + * Renesas Serial Communication Interface
> > + *
> > + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> > + * (Rev.1.40 R01UH0033EJ0140)
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/log.h"
> > +#include "qapi/error.h"
> > +#include "qemu-common.h"
> > +#include "cpu.h"
> > +#include "hw/hw.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/registerfields.h"
> > +#include "hw/char/renesas_sci.h"
> > +#include "qemu/error-report.h"
> > +
> > +/* SCI register map */
> > +REG8(SMR, 0)
> > +  FIELD(SMR, CKS,  0, 2)
> > +  FIELD(SMR, MP,   2, 1)
> > +  FIELD(SMR, STOP, 3, 1)
> > +  FIELD(SMR, PM,   4, 1)
> > +  FIELD(SMR, PE,   5, 1)
> > +  FIELD(SMR, CHR,  6, 1)
> > +  FIELD(SMR, CM,   7, 1)
> > +REG8(BRR, 1)
> > +REG8(SCR, 2)
> > +  FIELD(SCR, CKE, 0, 2)
> > +  FIELD(SCR, TEIE, 2, 1)
> > +  FIELD(SCR, MPIE, 3, 1)
> > +  FIELD(SCR, RE,   4, 1)
> > +  FIELD(SCR, TE,   5, 1)
> > +  FIELD(SCR, RIE,  6, 1)
> > +  FIELD(SCR, TIE,  7, 1)
> > +REG8(TDR, 3)
> > +REG8(SSR, 4)
> > +  FIELD(SSR, MPBT, 0, 1)
> > +  FIELD(SSR, MPB,  1, 1)
> > +  FIELD(SSR, TEND, 2, 1)
> > +  FIELD(SSR, ERR, 3, 3)
> > +FIELD(SSR, PER,  3, 1)
> > +FIELD(SSR, FER,  4, 1)
> > +FIELD(SSR, ORER, 5, 1)
> > +  FIELD(SSR, RDRF, 6, 1)
> > +  FIELD(SSR, TDRE, 7, 1)
> > +REG8(RDR, 5)
> > +REG8(SCMR, 6)
> > +  FIELD(SCMR, SMIF, 0, 1)
> > +  FIELD(SCMR, SINV, 2, 1)
> > +  FIELD(SCMR, SDIR, 3, 1)
> > +  FIELD(SCMR, BCP2, 7, 1)
> > +REG8(SEMR, 7)
> > +  FIELD(SEMR, ACS0, 0, 1)
> > +  FIELD(SEMR, ABCS, 4, 1)
> > +
> > +static int can_receive(void *opaque)
> > +{
> > +RSCIState *sci = RSCI(opaque);
> > +if (sci->rx_next > qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)) {
> > +return 0;
> > +} else {
> > +return FIELD_EX8(sci->scr, SCR, RE);
> > +}
> > +}
> > +
> > +static void receive(void *opaque, const uint8_t *buf, int size)
> > +{
> > +RSCIState *sci = RSCI(opaque);
> > +sci->rx_next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + sci->trtime;
> > +if (FIELD_EX8(sci->ssr, SSR, RDRF) || size > 1) {
> > +sci->ssr = 

Re: [Qemu-devel] [PATCH RFC v8 00/12] Add RX archtecture support

[Yoshinori Sato]

On Sat, 04 May 2019 01:11:48 +0900,
Alex Bennée wrote:
> 
> 
> Yoshinori Sato  writes:
> 
> > Hello.
> > This patch series is added Renesas RX target emulation.
> 
> I think the series is almost there - it's mostly just nits and clean
> build fixes to sort out now. If you run the branch through CI you will
> see where things fail to build.
> 
> However once that is all sorted it should be good for merging. I guess
> Richard would merge it until we can get your key signed so you can
> submit your own PRs:
> 
>   https://wiki.qemu.org/KeySigningParty
>

OK.
Should I tag it with "git tag -s"?

> --
> Alex Bennée
> 

-- 
Yosinori Sato

Re: [Qemu-devel] [PATCH RFC v8 12/12] hw/registerfields.h: Add 8bit and 16bit register macros.

[Yoshinori Sato]

On Sat, 04 May 2019 00:27:29 +0900,
Alex Bennée wrote:
> 
> 
> Yoshinori Sato  writes:
> 
> > Some RX peripheral using 8bit and 16bit registers.
> > Added 8bit and 16bit APIs.
> 
> Doesn't this mean the build breaks at some point? Features used by other
> patches should be introduced first so the build remains bisectable.

Hmm, It changes only added new macros.
So don't broken this changes.

> >
> > Signed-off-by: Yoshinori Sato 
> > ---
> >  include/hw/registerfields.h | 28 +++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
> > index 2659a58737..51bfd0cf67 100644
> > --- a/include/hw/registerfields.h
> > +++ b/include/hw/registerfields.h
> > @@ -22,6 +22,14 @@
> >  enum { A_ ## reg = (addr) };  \
> >  enum { R_ ## reg = (addr) / 4 };
> >
> > +#define REG8(reg, addr)  \
> > +enum { A_ ## reg = (addr) };  \
> > +enum { R_ ## reg = (addr) };
> > +
> > +#define REG16(reg, addr)  \
> > +enum { A_ ## reg = (addr) };  \
> > +enum { R_ ## reg = (addr) / 2 };
> > +
> >  /* Define SHIFT, LENGTH and MASK constants for a field within a register */
> >
> >  /* This macro will define R_FOO_BAR_MASK, R_FOO_BAR_SHIFT and 
> > R_FOO_BAR_LENGTH
> > @@ -40,6 +48,8 @@
> >  #define FIELD_EX64(storage, reg, field)   \
> >  extract64((storage), R_ ## reg ## _ ## field ## _SHIFT,   \
> >R_ ## reg ## _ ## field ## _LENGTH)
> > +#define FIELD_EX8  FIELD_EX32
> > +#define FIELD_EX16 FIELD_EX32
> 
> Hmm maybe we should be defining extract16/extract8 in bitops so things
> are a) properly types and b) bounds checked to catch errors.

I think so.
I will added extrat8 and extract16 in bitops.h.

> >
> >  /* Extract a field from an array of registers */
> >  #define ARRAY_FIELD_EX32(regs, reg, field)\
> > @@ -49,6 +59,22 @@
> >   * Assigning values larger then the target field will result in
> >   * compilation warnings.
> >   */
> > +#define FIELD_DP8(storage, reg, field, val) ({\
> > +struct {  \
> > +unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;\
> > +} v = { .v = val };   \
> > +uint8_t d;\
> > +d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,   \
> > +  R_ ## reg ## _ ## field ## _LENGTH, v.v);   \
> > +d; })
> > +#define FIELD_DP16(storage, reg, field, val) ({   \
> > +struct {  \
> > +unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;\
> > +} v = { .v = val };   \
> > +uint16_t d;   \
> > +d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,   \
> > +  R_ ## reg ## _ ## field ## _LENGTH, v.v);   \
> > +d; })
> >  #define FIELD_DP32(storage, reg, field, val) ({   \
> >  struct {  \
> >  unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;\
> > @@ -57,7 +83,7 @@
> >  d = deposit32((storage), R_ ## reg ## _ ## field ## _SHIFT,   \
> >R_ ## reg ## _ ## field ## _LENGTH, v.v);   \
> >  d; })
> > -#define FIELD_DP64(storage, reg, field, val) ({   \
> > +#define FIELD_DP64(storage, reg, field, val) ({ \
> >  struct {  \
> >  unsigned int v:R_ ## reg ## _ ## field ## _LENGTH;\
> >  } v = { .v = val };   \
> 
> 
> --
> Alex Bennée
> 

-- 
Yosinori Sato

Re: [Qemu-devel] [PATCH RFC v8 01/12] target/rx: TCG translation

[Yoshinori Sato]

On Sat, 04 May 2019 03:43:23 +0900,
Richard Henderson wrote:
> 
> On 5/2/19 7:33 AM, Yoshinori Sato wrote:
> > +/* conditional branch helper */
> > +static void rx_bcnd_main(DisasContext *ctx, int cd, int dst)
> > +{
> > +DisasCompare dc;
> > +TCGLabel *t, *done;
> > +
> > +switch (cd) {
> > +case 0 ... 13:
> > +dc.temp = tcg_temp_new();
> > +psw_cond(, cd);
> > +t = gen_new_label();
> > +done = gen_new_label();
> > +tcg_gen_brcondi_i32(dc.cond, dc.value, 0, t);
> > +gen_goto_tb(ctx, 0, ctx->base.pc_next);
> > +tcg_gen_br(done);
> > +gen_set_label(t);
> > +gen_goto_tb(ctx, 1, ctx->pc + dst);
> > +gen_set_label(done);
> > +tcg_temp_free(dc.temp);
> > +break;
> > +case 14:
> > +/* always true case */
> > +gen_goto_tb(ctx, 0, ctx->pc + dst);
> > +break;
> > +case 15:
> > +/* always false case */
> > +/* Nothing do */
> > +break;
> > +}
> > +ctx->base.is_jmp = DISAS_JUMP;
> > +}
> 
> Do not set is_jmp to DISAS_JUMP here.  We have already set is_jmp to
> DISAS_NORETURN in gen_goto_tb.  For case 15, we do not need to exit the TB in
> order to treat the never-taken branch as a nop.
> 
> This assignment means that we will emit *another* exit from the TB in
> rx_tr_tb_stop, which will be unreachable code.
> 
> This is the only bug I see in this revision.  Thanks for your patience!
>

OK.
I did not notice it because it was never used.

Too many thanks.

> 
> r~
> 

-- 
Yosinori Sato

Re: [Qemu-devel] [PATCH RFC v8 05/12] target/rx: Miscellaneous files

[Yoshinori Sato]

On Sat, 04 May 2019 01:06:44 +0900,
Alex Bennée wrote:
> 
> 
> Yoshinori Sato  writes:
> 
> > Signed-off-by: Yoshinori Sato 
> > ---
> >  target/rx/gdbstub.c | 112 
> > 
> >  target/rx/monitor.c |  38 
> >  target/rx/Makefile.objs |  11 +
> >  3 files changed, 161 insertions(+)
> >  create mode 100644 target/rx/gdbstub.c
> >  create mode 100644 target/rx/monitor.c
> >  create mode 100644 target/rx/Makefile.objs
> >
> > diff --git a/target/rx/gdbstub.c b/target/rx/gdbstub.c
> > new file mode 100644
> > index 00..d76ca52e82
> > --- /dev/null
> > +++ b/target/rx/gdbstub.c
> > @@ -0,0 +1,112 @@
> > +/*
> > + * RX gdb server stub
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +#include "qemu/osdep.h"
> > +#include "qemu-common.h"
> > +#include "cpu.h"
> > +#include "exec/gdbstub.h"
> > +
> > +int rx_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
> > +{
> > +RXCPU *cpu = RXCPU(cs);
> > +CPURXState *env = >env;
> > +
> > +switch (n) {
> > +case 0 ... 15:
> > +return gdb_get_regl(mem_buf, env->regs[n]);
> > +case 16:
> > +return gdb_get_regl(mem_buf, (env->psw_u) ? env->regs[0] : 
> > env->usp);
> > +case 17:
> > +return gdb_get_regl(mem_buf, (!env->psw_u) ? env->regs[0] : 
> > env->isp);
> > +case 18:
> > +return gdb_get_regl(mem_buf, rx_cpu_pack_psw(env));
> > +case 19:
> > +return gdb_get_regl(mem_buf, env->pc);
> > +case 20:
> > +return gdb_get_regl(mem_buf, env->intb);
> > +case 21:
> > +return gdb_get_regl(mem_buf, env->bpsw);
> > +case 22:
> > +return gdb_get_regl(mem_buf, env->bpc);
> > +case 23:
> > +return gdb_get_regl(mem_buf, env->fintv);
> > +case 24:
> > +return gdb_get_regl(mem_buf, env->fpsw);
> > +case 25:
> > +return 0;
> > +}
> > +return 0;
> > +}
> > +
> > +int rx_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
> > +{
> > +RXCPU *cpu = RXCPU(cs);
> > +CPURXState *env = >env;
> > +uint32_t psw;
> > +switch (n) {
> > +case 0 ... 15:
> > +env->regs[n] = ldl_p(mem_buf);
> > +if (n == 0) {
> > +if (env->psw_u) {
> > +env->usp = env->regs[0];
> > +} else {
> > +env->isp = env->regs[0];
> > +}
> > +}
> > +break;
> > +case 16:
> > +env->usp = ldl_p(mem_buf);
> > +if (env->psw_u) {
> > +env->regs[0] = ldl_p(mem_buf);
> > +}
> > +break;
> > +case 17:
> > +env->isp = ldl_p(mem_buf);
> > +if (!env->psw_u) {
> > +env->regs[0] = ldl_p(mem_buf);
> > +}
> > +break;
> > +case 18:
> > +psw = ldl_p(mem_buf);
> > +rx_cpu_unpack_psw(env, psw, 1);
> > +break;
> > +case 19:
> > +env->pc = ldl_p(mem_buf);
> > +break;
> > +case 20:
> > +env->intb = ldl_p(mem_buf);
> > +break;
> > +case 21:
> > +env->bpsw = ldl_p(mem_buf);
> > +break;
> > +case 22:
> > +env->bpc = ldl_p(mem_buf);
> > +break;
> > +case 23:
> > +env->fintv = ldl_p(mem_buf);
> > +break;
> > +case 24:
> > +env->fpsw = ldl_p(mem_buf);
> > +break;
> > +case 25:
> > +return 8;
> > +default:
> > +return 0;
> > +}
> > +
> > +return 4;
> > +}
> > diff --git a/target/rx/monitor.c b/target/rx/monitor.c
> > new file mode 100644
> > index 00..5d7a1e58b5
> > --- /dev/null
> > +++ b/target/rx/monitor.c
> > @@ -0,0 +1,38 @@
> > +/*
> > + * QEMU monitor
> > + *
> > + * Copyright (c) 2003-2004 Fabrice Bellard
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above 

Re: [Qemu-devel] [PATCH RFC v8 09/12] hw/rx: RX Target hardware definition

[Yoshinori Sato]

On Sat, 04 May 2019 00:38:38 +0900,
Alex Bennée wrote:
> 
> 
> Yoshinori Sato  writes:
> 
> > rx62n - RX62N cpu.
> > rxqemu - QEMU virtual target.
> >
> > Signed-off-by: Yoshinori Sato 
> > ---
> >  include/hw/rx/rx.h|   7 ++
> >  include/hw/rx/rx62n.h |  54 
> >  hw/rx/rx62n.c | 226 
> > ++
> >  hw/rx/rxqemu.c| 100 ++
> >  hw/rx/Kconfig |   2 +
> >  hw/rx/Makefile.objs   |   1 +
> >  6 files changed, 390 insertions(+)
> >  create mode 100644 include/hw/rx/rx.h
> >  create mode 100644 include/hw/rx/rx62n.h
> >  create mode 100644 hw/rx/rx62n.c
> >  create mode 100644 hw/rx/rxqemu.c
> >  create mode 100644 hw/rx/Kconfig
> >  create mode 100644 hw/rx/Makefile.objs
> >
> > diff --git a/include/hw/rx/rx.h b/include/hw/rx/rx.h
> > new file mode 100644
> > index 00..ff5924b81f
> > --- /dev/null
> > +++ b/include/hw/rx/rx.h
> > @@ -0,0 +1,7 @@
> > +#ifndef QEMU_RX_H
> > +#define QEMU_RX_H
> > +/* Definitions for RX board emulation.  */
> > +
> > +#include "target/rx/cpu-qom.h"
> > +
> > +#endif
> > diff --git a/include/hw/rx/rx62n.h b/include/hw/rx/rx62n.h
> > new file mode 100644
> > index 00..8c15399ce0
> > --- /dev/null
> > +++ b/include/hw/rx/rx62n.h
> > @@ -0,0 +1,54 @@
> > +/*
> > + * RX62N Object
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +
> > +#ifndef HW_RX_RX62N_H
> > +#define HW_RX_RX62N_H
> > +
> > +#include "hw/sysbus.h"
> > +#include "hw/rx/rx.h"
> > +#include "hw/intc/rx_icu.h"
> > +#include "hw/timer/renesas_tmr.h"
> > +#include "hw/timer/renesas_cmt.h"
> > +#include "hw/char/renesas_sci.h"
> > +
> > +#define TYPE_RX62N "rx62n"
> > +#define TYPE_RX62N_CPU RX_CPU_TYPE_NAME(TYPE_RX62N)
> > +#define RX62N(obj) OBJECT_CHECK(RX62NState, (obj), TYPE_RX62N)
> > +
> > +typedef struct RX62NState {
> > +SysBusDevice parent_obj;
> > +
> > +RXCPU *cpu;
> > +RXICUState *icu;
> > +RTMRState *tmr[2];
> > +RCMTState *cmt[2];
> > +RSCIState *sci[6];
> > +
> > +MemoryRegion *sysmem;
> > +bool kernel;
> > +
> > +MemoryRegion iram;
> > +MemoryRegion iomem1;
> > +MemoryRegion d_flash;
> > +MemoryRegion iomem2;
> > +MemoryRegion iomem3;
> > +MemoryRegion c_flash;
> > +qemu_irq irq[256];
> > +} RX62NState;
> > +
> > +#endif
> > diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c
> > new file mode 100644
> > index 00..b303aefe8c
> > --- /dev/null
> > +++ b/hw/rx/rx62n.c
> > @@ -0,0 +1,226 @@
> > +/*
> > + * RX62N device
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qapi/error.h"
> > +#include "hw/hw.h"
> > +#include "hw/rx/rx62n.h"
> > +#include "hw/loader.h"
> > +#include "hw/sysbus.h"
> > +#include "sysemu/sysemu.h"
> > +#include "cpu.h"
> > +#include "exec/exec-all.h"
> > +#include "exec/address-spaces.h"
> > +
> > +static const int ipr_table[] = {
> 
> This could probably do with a little comment above explaining what the
> magic table is and where the data in it is referenced.

OK. Added comment.

> Is there any particular reason this is exposed as a property? That could
> be explained as well.

As this has a different value for each CPU type,
I think that it is appropriate to prepare for CPU instead of INTC.

> > +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
> > +0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, /* 15 */
> > +0x00, 0xff, 0xff, 0xff, 0xff, 0x01, 0xff, 0x02,
> > +0xff, 0xff, 0xff, 0x03, 0x04, 0x05, 0x06, 0x07, /* 31 */
> > +0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f,
> > +0x10, 0x11, 

Re: [Qemu-devel] [PATCH RFC v8 07/12] hw/timer: RX62N internal timer modules

[Yoshinori Sato]

On Sat, 04 May 2019 00:20:47 +0900,
Alex Bennée wrote:
> 
> 
> Yoshinori Sato  writes:
> 
> > renesas_tmr: 8bit timer modules.
> > renesas_cmt: 16bit compare match timer modules.
> > This part use many renesas's CPU.
> > Hardware manual.
> > https://www.renesas.com/us/en/doc/products/mpumcu/doc/rx_family/r01uh0033ej0140_rx62n.pdf
> >
> > Signed-off-by: Yoshinori Sato 
> > ---
> >  include/hw/timer/renesas_cmt.h |  33 +++
> >  include/hw/timer/renesas_tmr.h |  46 +
> >  hw/timer/renesas_cmt.c | 277 +
> >  hw/timer/renesas_tmr.c | 458 
> > +
> >  hw/timer/Kconfig   |   6 +
> >  hw/timer/Makefile.objs |   3 +
> >  6 files changed, 823 insertions(+)
> >  create mode 100644 include/hw/timer/renesas_cmt.h
> >  create mode 100644 include/hw/timer/renesas_tmr.h
> >  create mode 100644 hw/timer/renesas_cmt.c
> >  create mode 100644 hw/timer/renesas_tmr.c
> >
> > diff --git a/include/hw/timer/renesas_cmt.h b/include/hw/timer/renesas_cmt.h
> > new file mode 100644
> > index 00..7e393d7ad3
> > --- /dev/null
> > +++ b/include/hw/timer/renesas_cmt.h
> > @@ -0,0 +1,33 @@
> > +/*
> > + * Renesas Compare-match timer Object
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This code is licensed under the GPL version 2 or later.
> > + *
> > + */
> > +
> > +#ifndef HW_RENESAS_CMT_H
> > +#define HW_RENESAS_CMT_H
> > +
> > +#include "hw/sysbus.h"
> > +
> > +#define TYPE_RENESAS_CMT "renesas-cmt"
> > +#define RCMT(obj) OBJECT_CHECK(RCMTState, (obj), TYPE_RENESAS_CMT)
> > +
> > +typedef struct RCMTState {
> > +SysBusDevice parent_obj;
> > +
> > +uint64_t input_freq;
> > +MemoryRegion memory;
> > +
> > +uint16_t cmstr;
> > +uint16_t cmcr[2];
> > +uint16_t cmcnt[2];
> > +uint16_t cmcor[2];
> > +int64_t tick[2];
> > +qemu_irq cmi[2];
> > +QEMUTimer *timer[2];
> > +} RCMTState;
> > +
> > +#endif
> > diff --git a/include/hw/timer/renesas_tmr.h b/include/hw/timer/renesas_tmr.h
> > new file mode 100644
> > index 00..718d9dc4ff
> > --- /dev/null
> > +++ b/include/hw/timer/renesas_tmr.h
> > @@ -0,0 +1,46 @@
> > +/*
> > + * Renesas 8bit timer Object
> > + *
> > + * Copyright (c) 2018 Yoshinori Sato
> > + *
> > + * This code is licensed under the GPL version 2 or later.
> > + *
> > + */
> > +
> > +#ifndef HW_RENESAS_TMR_H
> > +#define HW_RENESAS_TMR_H
> > +
> > +#include "hw/sysbus.h"
> > +
> > +#define TYPE_RENESAS_TMR "renesas-tmr"
> > +#define RTMR(obj) OBJECT_CHECK(RTMRState, (obj), TYPE_RENESAS_TMR)
> > +
> > +enum timer_event {cmia = 0,
> > +  cmib = 1,
> > +  ovi = 2,
> > +  none = 3,
> > +  TMR_NR_EVENTS = 4};
> > +enum {CH = 2};
> > +typedef struct RTMRState {
> > +SysBusDevice parent_obj;
> > +
> > +uint64_t input_freq;
> > +MemoryRegion memory;
> > +
> > +uint8_t tcnt[CH];
> > +uint8_t tcora[CH];
> > +uint8_t tcorb[CH];
> > +uint8_t tcr[CH];
> > +uint8_t tccr[CH];
> > +uint8_t tcor[CH];
> > +uint8_t tcsr[CH];
> > +int64_t tick;
> > +int64_t div_round[CH];
> > +enum timer_event next[CH];
> > +qemu_irq cmia[CH];
> > +qemu_irq cmib[CH];
> > +qemu_irq ovi[CH];
> > +QEMUTimer *timer[CH];
> > +} RTMRState;
> > +
> > +#endif
> > diff --git a/hw/timer/renesas_cmt.c b/hw/timer/renesas_cmt.c
> > new file mode 100644
> > index 00..b82250dbc2
> > --- /dev/null
> > +++ b/hw/timer/renesas_cmt.c
> > @@ -0,0 +1,277 @@
> > +/*
> > + * Renesas 16bit Compare-match timer
> > + *
> > + * Datasheet: RX62N Group, RX621 Group User's Manual: Hardware
> > + * (Rev.1.40 R01UH0033EJ0140)
> > + *
> > + * Copyright (c) 2019 Yoshinori Sato
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License 
> > for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along 
> > with
> > + * this program.  If not, see .
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu-common.h"
> > +#include "qemu/log.h"
> > +#include "qapi/error.h"
> > +#include "qemu/timer.h"
> > +#include "cpu.h"
> > +#include "hw/hw.h"
> > +#include "hw/sysbus.h"
> > +#include "hw/registerfields.h"
> > +#include "hw/timer/renesas_cmt.h"
> > +#include "qemu/error-report.h"
> > +
> > +/*
> > + *  +0 CMSTR - common control
> > + *  +2 CMCR  - ch0
> > + *  +4 CMCNT - ch0
> > + *  +6 CMCOR - ch0
> > + *  +8 CMCR  - ch1
> > + * +10 CMCNT - ch1
> > + * +12 CMCOR - ch1
> > + * If we think 

Re: [Qemu-devel] [RFC PATCH] tests/qemu-iotests: re-format output to for make check-block

[Thomas Huth]

On 03/05/2019 16.39, Alex Bennée wrote:
> This attempts to clean-up the output to better match the output of the
> rest of the QEMU check system. This includes:
> 
>   - formatting as "  TESTiotest: nnn"
>   - calculating time diff at the end
>   - only dumping config on failure
> 
> Signed-off-by: Alex Bennée 
> ---
>  tests/qemu-iotests/check | 71 +++-
>  1 file changed, 34 insertions(+), 37 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 922c5d1d3d..2ffc14113e 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -633,12 +633,6 @@ _wallclock()
>  date "+%H %M %S" | awk '{ print $1*3600 + $2*60 + $3 }'
>  }
>  
> -_timestamp()
> -{
> -now=$(date "+%T")
> -printf %s " [$now]"
> -}
> -
>  _wrapup()
>  {
>  if $showme
> @@ -709,19 +703,6 @@ trap "_wrapup; exit \$status" 0 1 2 3 15
>  FULL_IMGFMT_DETAILS=$(_full_imgfmt_details)
>  FULL_HOST_DETAILS=$(_full_platform_details)
>  
> -cat < -QEMU  -- "$QEMU_PROG" $QEMU_OPTIONS
> -QEMU_IMG  -- "$QEMU_IMG_PROG" $QEMU_IMG_OPTIONS
> -QEMU_IO   -- "$QEMU_IO_PROG" $QEMU_IO_OPTIONS
> -QEMU_NBD  -- "$QEMU_NBD_PROG" $QEMU_NBD_OPTIONS
> -IMGFMT-- $FULL_IMGFMT_DETAILS
> -IMGPROTO  -- $IMGPROTO
> -PLATFORM  -- $FULL_HOST_DETAILS
> -TEST_DIR  -- $TEST_DIR
> -SOCKET_SCM_HELPER -- $SOCKET_SCM_HELPER
> -
> -EOF

Maybe turn it into a function instead, so that it could also always be
printed when the script is run with the "-v" parameter?

>  seq="check"
>  
>  [ -n "$TESTS_REMAINING_LOG" ] && echo $list > $TESTS_REMAINING_LOG
> @@ -729,7 +710,9 @@ seq="check"
>  for seq in $list
>  do
>  err=false
> -printf %s "$seq"
> +reason=""
> +times=""
> +
>  if [ -n "$TESTS_REMAINING_LOG" ] ; then
>  sed -e "s/$seq//" -e 's/  / /' -e 's/^ *//' $TESTS_REMAINING_LOG > 
> $TESTS_REMAINING_LOG.tmp
>  mv $TESTS_REMAINING_LOG.tmp $TESTS_REMAINING_LOG
> @@ -738,7 +721,7 @@ do
>  
>  if $showme
>  then
> -echo
> +echo "  TESTiotest: $seq (not actually run)"

I wonder whether some other scripts depend on the output of "check -n"
... in that case, it make sense to only print the numbers, without the
additional strings here.

>  continue
>  elif [ -f expunged ] && $expunge && egrep "^$seq([ ]|\$)" 
> expunged >/dev/null
>  then
> @@ -753,17 +736,11 @@ do
>  # really going to try and run this one
>  #
>  rm -f $seq.out.bad
> -lasttime=$(sed -n -e "/^$seq /s/.* //p" <$TIMESTAMP_FILE)
> -if [ "X$lasttime" != X ]; then
> -printf %s " ${lasttime}s ..."
> -else
> -printf ""# prettier output with timestamps.
> -fi
>  rm -f core $seq.notrun
>  rm -f $seq.casenotrun
>  
>  start=$(_wallclock)
> -$timestamp && printf %s "[$(date "+%T")]"
> +$timestamp && times="[$(date "+%T")]"
>  
>  if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env 
> python" ]; then
>  run_command="$PYTHON $seq"
> @@ -781,26 +758,26 @@ do
>  $run_command >$tmp.out 2>&1)
>  fi
>  sts=$?
> -$timestamp && _timestamp
> +$timestamp && times="$times -> [$(date "+%T")]"
>  stop=$(_wallclock)
>  
>  if [ -f core ]
>  then
> -printf " [dumped core]"
>  mv core $seq.core
> +reason="dumped core $seq.core"
>  err=true
>  fi
>  
>  if [ -f $seq.notrun ]
>  then
> -$timestamp || printf " [not run] "
> -$timestamp && echo " [not run]" && printf %s "$seq -- "
> +$timestamp || reason="[not run]"
> +$timestamp && reason="[not run] $seq -- "
>  cat $seq.notrun
>  notrun="$notrun $seq"
>  else
>  if [ $sts -ne 0 ]
>  then
> -printf %s " [failed, exit status $sts]"
> +reason=$(printf %s "[failed, exit status $sts]")
>  err=true
>  fi
>  
> @@ -821,22 +798,27 @@ do
>  
>  if [ ! -f "$reference" ]
>  then
> -echo " - no qualified output"
> +reason=" - no qualified output"
>  err=true
>  else
>  if diff -w "$reference" $tmp.out >/dev/null 2>&1
>  then
> -echo ""
>  if $err
>  then
>  :
>  else
> -echo "$seq $(expr $stop - $start)" >>$tmp.time
> +lasttime=$(sed -n -e "/^$seq /s/.* //p" 
> <$TIMESTAMP_FILE)
> +thistime=$(expr $stop - $start)
> +echo "$seq $thistime" >>$tmp.time
> +
> +if [ 

Re: [Qemu-devel] [PATCH 02/14] target/ppc: remove getVSR()/putVSR() from mem_helper.c

[Richard Henderson]

On 5/5/19 8:49 AM, Mark Cave-Ayland wrote:
> Okay in that case I'll leave it as-is. So just to satisfy my curiosity here: 
> is the
> problem here the mixing and matching of offsets and TCG globals, rather than 
> the use
> of offsets as done for the VMX/VSX registers?

Correct.


r~

Re: [Qemu-devel] [PATCH v1 8/8] target/avr: Register AVR support with the rest of QEMU, the build system, and the MAINTAINERS file

[Richard Henderson]

On 5/4/19 1:36 AM, Sarah Harris wrote:
> Signed-off-by: Sarah Harris 
...
>  
> +AVR
> +M: Michael Rolnik 
> +S: Odd Fixes
> +F: target-avr/
> +F: hw/avr/
> +

This is not how things work.  Michael wasn't up to maintaining the code 2 years
ago; that's why it was never committed.

You would need to maintain this yourself, and for more than "Odd Fixes".


r~

Re: [Qemu-devel] [RFC PATCH] tests/qemu-iotests: re-format output to for make check-block

[Thomas Huth]

On 03/05/2019 18.15, Alex Bennée wrote:
> 
> Thomas Huth  writes:
> 
>> On 03/05/2019 16.39, Alex Bennée wrote:
>>> This attempts to clean-up the output to better match the output of the
>>> rest of the QEMU check system. This includes:
>>>
>>>   - formatting as "  TESTiotest: nnn"
>>>   - calculating time diff at the end
>>>   - only dumping config on failure
>>>
>>> Signed-off-by: Alex Bennée 
>>> ---
>>>  tests/qemu-iotests/check | 71 +++-
>>>  1 file changed, 34 insertions(+), 37 deletions(-)
>>
>> Thanks for tackling this! The output now looks nicer indeed if you run
>> "make check-qtest check-block -j8". However, if you add a "V=1" at the
>> end of the command line, the outputs look quite different again...
>>
>> That's why I thought that having a TAP mode for the check script could
>> be a good idea, too. Then we could pipe the output through the
>> tap-driver.pl script, too, so we get uniform output for all tests...?
> 
> That would probably be a cleaner approach. What would be even better is
> somehow expanding the list of tests at make time so you could run your
> tests in parallel.

I agree that this might be the ultimate solution ... but I'm not sure
whether the iotests are really ready for being run in parallel yet, so
it will likely take quite some while 'till we are at that point. With
that in mind (and thus also not sure yet whether my TAP idea is really
the right approach), your patch is certainly a good interim solution
which we should try to get merged, too, when my "make check" series gets
accepted?

> I did wonder how useful the timing stuff was to developers.

Yes, me too ... maybe the block layer folks can comment on that one...?

 Thomas

Re: [Qemu-devel] [PATCH 14/14] target/ppc: improve VSX_FMADD with new GEN_VSX_HELPER_VSX_MADD macro

[Mark Cave-Ayland]

On 05/05/2019 16:17, Richard Henderson wrote:

> On 5/5/19 3:20 AM, Mark Cave-Ayland wrote:
>>> The afrm argument is no longer used.
>>> This also means that e.g.
>>>
>>> VSX_MADD(xsmaddadp, 1, float64, VsrD(0), MADD_FLGS, 1, 1, 0)
>>> VSX_MADD(xsmaddmdp, 1, float64, VsrD(0), MADD_FLGS, 0, 1, 0)
>>>
>>> are redundant.  Similarly with all of the other pairs.
>>
>> Agreed. What do you think is the best solution here - maybe a double macro 
>> that looks
>> something like this?
>>
>> #define VSX_MADD(op, prec, nels, tp, fld, maddflgs, sfprf, r2sp)
>> _VSX_MADD(op##aprec, nels, tp, fld, maddflgs, sfprf, r2sp)
>> _VSX_MADD(op##mprec, nels, tp, fld, maddflgs, sfprf, r2sp)
>>
>> VSX_MADD(xsmadd, dp, 1, float64, VsrD(0), MADD_FLGS, 1, 0)
> 
> I have no idea what you're suggesting.
> 
> I am suggesting one function, xsmadddp, replacing xsmaddadp + xsmaddmdp, that
> is used by both instructions.

Gotcha. I was thinking that the standard practice was to have a 1:1 
correspondence
between the gen function and helper function for instructions, but I'm happy to 
go
with your suggestion above.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 02/14] target/ppc: remove getVSR()/putVSR() from mem_helper.c

[Mark Cave-Ayland]

On 05/05/2019 15:34, Richard Henderson wrote:

> On 5/5/19 2:34 AM, Mark Cave-Ayland wrote:
  EA = tcg_temp_new();\
 -xt = tcg_const_tl(xT(ctx->opcode)); \
  gen_set_access_type(ctx, ACCESS_INT);   \
  gen_addr_register(ctx, EA); \
 -gen_helper_##name(cpu_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
 +xt = tcg_const_tl(xT(ctx->opcode)); \
 +rb = tcg_const_tl(rB(ctx->opcode)); \
 +gen_helper_##name(cpu_env, EA, xt, rb); \
  tcg_temp_free(EA);  \
  tcg_temp_free(xt);  \
 +tcg_temp_free(rb);  \
  }
>>>
>>> Why are you adjusting the function to pass the rB register number rather 
>>> than
>>> the contents of rB?  That seems the wrong way around...
>>
>> I think what I was trying to do here was eliminate the cpu_gpr since it
>> feels to me that with the vector patchsets and your negative offset patches
>> that this should be the way to go for accessing CPUState rather than using
>> TCG globals.
> 
> Not for the integer register set.
> 
>> Looking at this again I realise the solution is really the same as is
>> currently used
>> for gen_load_spr() so I can use something like this:>
>> static inline void gen_load_gpr(TCGv t, int reg)
>> {
>> tcg_gen_ld_tl(t, cpu_env, offsetof(CPUPPCState, gpr[reg]));
>> }
>>
>> Does this seem reasonable as a solution?
> 
> No, this will fail quickly.

Okay in that case I'll leave it as-is. So just to satisfy my curiosity here: is 
the
problem here the mixing and matching of offsets and TCG globals, rather than 
the use
of offsets as done for the VMX/VSX registers?


ATB,

Mark.

Re: [Qemu-devel] [PATCH v1 1/8] target/avr: Add instruction decoder

[Richard Henderson]

On 5/4/19 1:36 AM, Sarah Harris wrote:
> This utility module builds a decision tree to decode instructions, starting 
> from a human readable list of instruction bit patterns.
> Automatic tree generation will hopefully be more efficient and more 
> maintainable than a hand-designed opcode parser.
> 
> Tree generation happens at startup because this seemed simpler to implement 
> than adding a new build step.

We have such a thing in qemu already, as a separate build step.

See ./scripts/decodetree.py, and some of the uses in

  target/{arm,hppa,riscv}/*.decode

In addition to being able to select the instruction, it also
extracts arguments from the instruction, so there's less
repetition that you have for e.g.

 > +static inline uint32_t MOVW_Rr(uint32_t opcode)
> +{
> +return extract32(opcode, 0, 4);
> +}
...
> +static inline uint32_t MULS_Rr(uint32_t opcode)
> +{
> +return extract32(opcode, 0, 4);
> +}


r~

Re: [Qemu-devel] [PATCH 01/14] target/ppc: remove getVSR()/putVSR() from fpu_helper.c

[Mark Cave-Ayland]

On 05/05/2019 15:31, Richard Henderson wrote:

> On 5/5/19 2:27 AM, Mark Cave-Ayland wrote:
>> I've spent a bit of time today going through the functions and it seems that 
>> all
>> functions which have an xt parameter, minus a couple of the TEST macros, 
>> require the
>> result to be calculated in a local variable first.
>>
>> I think the best solution is still to remove getVSR()/putVSR() but replace 
>> them with
>> macros for copying and zeroing like this:
>>
>> #define VSRCPY(d, s) (memcpy(d, s, sizeof(ppc_vsr_t)))
>> #define VSRZERO(d)   (memset(d, 0, sizeof(ppc_vsr_t)))
> 
> Local variable, yes.  But I see no reason for macros.
> 
>   ppc_vsr_t res = { };
>   ...
>   *xt = res;

Ah thanks for the hint - I wasn't aware that this could be done in a consistent 
way
for structs across all compilers. I'll implement this in the next version of 
the series.


ATB,

Mark.

Re: [Qemu-devel] [PATCH 0/9] Assembly coroutine backend and x86 CET support

[Alex Bennée]

Paolo Bonzini  writes:

> *** BLURB HERE ***

I assume there was going to be a bit more background here?

--
Alex Bennée

Re: [Qemu-devel] [PATCH] pflash: Only read non-zero parts of backend image

[Peter Maydell]

On Sun, 5 May 2019 at 08:02, Xiang Zheng  wrote:
>
> Currently we fill the memory space with two 64MB NOR images when
> using persistent UEFI variables on virt board. Actually we only use
> a very small(non-zero) part of the memory while the rest significant
> large(zero) part of memory is wasted.
>
> So this patch checks the block status and only writes the non-zero part
> into memory. This requires pflash devices to use sparse files for
> backends.

Do you mean "pflash devices will no longer work if the file
that is backing them is not sparse", or just "if the file that
is backing them is not sparse then you won't get the benefit
of using less memory" ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 5/5] hw/arm: Add the Netduino Plus 2

[Peter Maydell]

On Sat, 4 May 2019 at 06:26, Alistair Francis  wrote:
> Ah, it seems like -device loader doesn't work, it looks like not
> setting the thumb register causes this core dump:
>
> qemu: fatal: Lockup: can't escalate 3 to HardFault (current priority -1)
>
> R00=2000 R01=0574 R02=200015d0 R03=200015d0
> R04= R05= R06= R07=
> R08= R09= R10= R11=
> R12= R13=ffe0 R14=fff9 R15=0800cba4

Is the ELF file incorrectly setting the entry point address to not
be a Thumb one (ie low bit set), or is the loader device not honouring
that? (I thought that we'd fixed the latter of those recently...)

thanks
-- PMM

[Qemu-devel] [PATCH 2/3] hw/ppc/40p: Move the MC146818 RTC to the board where it belongs

[Philippe Mathieu-Daudé]

The MC146818 RTC was incorrectly added to the i82378 chipset in
commit a04ff940974a. In the next commit (506b7ddf8893) the PReP
machine use the i82378.
Since the MC146818 is specific to the PReP machine, move its use
there.

Fixes: a04ff940974a
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/isa/i82378.c | 4 
 hw/ppc/prep.c   | 3 +++
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index a5d67bc6d7a..c08970b24a2 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -21,7 +21,6 @@
 #include "hw/pci/pci.h"
 #include "hw/i386/pc.h"
 #include "hw/timer/i8254.h"
-#include "hw/timer/mc146818rtc.h"
 #include "hw/audio/pcspk.h"
 
 #define TYPE_I82378 "i82378"
@@ -105,9 +104,6 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
 
 /* 2 82C37 (dma) */
 isa = isa_create_simple(isabus, "i82374");
-
-/* timer */
-isa_create_simple(isabus, TYPE_MC146818_RTC);
 }
 
 static void i82378_init(Object *obj)
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index ebee3211480..7a0d311d43c 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -675,6 +675,9 @@ static void ibm_40p_init(MachineState *machine)
 qdev_prop_set_uint32(dev, "ram-size", machine->ram_size);
 qdev_init_nofail(dev);
 
+/* RTC */
+isa_create_simple(isa_bus, TYPE_MC146818_RTC);
+
 /* initialize CMOS checksums */
 cmos_checksum = 0x6aa9;
 qbus_walk_children(BUS(isa_bus), prep_set_cmos_checksum, NULL, NULL, NULL,
-- 
2.20.1

[Qemu-devel] [PATCH 1/3] hw/ppc/prep: use TYPE_MC146818_RTC instead of a hardcoded string

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ppc/prep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index b7f459d4754..ebee3211480 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -601,7 +601,7 @@ static int prep_set_cmos_checksum(DeviceState *dev, void 
*opaque)
 uint16_t checksum = *(uint16_t *)opaque;
 ISADevice *rtc;
 
-if (object_dynamic_cast(OBJECT(dev), "mc146818rtc")) {
+if (object_dynamic_cast(OBJECT(dev), TYPE_MC146818_RTC)) {
 rtc = ISA_DEVICE(dev);
 rtc_set_memory(rtc, 0x2e, checksum & 0xff);
 rtc_set_memory(rtc, 0x3e, checksum & 0xff);
-- 
2.20.1

[Qemu-devel] [PATCH 3/3] hw/ppc/40p: use 1900 as a base year

[Philippe Mathieu-Daudé]

From: Artyom Tarasenko 

AIX 5.1 expects the base year to be 1900. Adjust accordingly.

Signed-off-by: Artyom Tarasenko 
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ppc/prep.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 7a0d311d43c..2a8009e20b4 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -676,7 +676,9 @@ static void ibm_40p_init(MachineState *machine)
 qdev_init_nofail(dev);
 
 /* RTC */
-isa_create_simple(isa_bus, TYPE_MC146818_RTC);
+dev = DEVICE(isa_create(isa_bus, TYPE_MC146818_RTC));
+qdev_prop_set_int32(dev, "base_year", 1900);
+qdev_init_nofail(dev);
 
 /* initialize CMOS checksums */
 cmos_checksum = 0x6aa9;
-- 
2.20.1

[Qemu-devel] [PATCH 0/3] hw/ppc/40p: Move the MC146818 RTC to the board where it belongs

[Philippe Mathieu-Daudé]

Hi,

This series is to properly do the fix sent by Artyom here:
https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02264.html

There is no RTC on the i82378, move it to the board code,
set the base year there.

Regards,

Phil.

Artyom Tarasenko (1):
  hw/ppc/40p: use 1900 as a base year

Philippe Mathieu-Daudé (2):
  hw/ppc/prep: use TYPE_MC146818_RTC instead of a hardcoded string
  hw/ppc/40p: Move the MC146818 RTC to the board where it belongs

 hw/isa/i82378.c | 4 
 hw/ppc/prep.c   | 7 ++-
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.20.1

Re: [Qemu-devel] [PATCH v2 3/3] hw/isa/i82378.c: use 1900 as a base year

[Philippe Mathieu-Daudé]

Hi Mark, Artyom.

On 5/5/19 12:46 PM, Mark Cave-Ayland wrote:
> On 04/05/2019 22:02, Artyom Tarasenko wrote:
> 
>> AIX 5.1 expects the base year to be 1900. Adjust accordingly.
>>
>> Signed-off-by: Artyom Tarasenko 
>> Reviewed-by: Hervé Poussineau 
>> ---
>>  hw/isa/i82378.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
>> index a5d67bc..546c928 100644
>> --- a/hw/isa/i82378.c
>> +++ b/hw/isa/i82378.c
>> @@ -107,7 +107,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>>  isa = isa_create_simple(isabus, "i82374");
>>  
>>  /* timer */
>> -isa_create_simple(isabus, TYPE_MC146818_RTC);
>> +isa = isa_create(isabus, TYPE_MC146818_RTC);
>> +qdev_prop_set_int32(DEVICE(isa), "base_year", 1900);
>> +qdev_init_nofail(DEVICE(isa));
>>  }
>>  
>>  static void i82378_init(Object *obj)
> 
> Is this true for the 82378 in general, or is it a particular quirk of the 40p
> machine/PReP specification?

This is indeed incorrect (see
https://lists.gnu.org/archive/html/qemu-devel/2019-04/msg02452.html) so:
Nack

Artyom: I did wrote the patch and included it in another series based on
top of Joel Stanley's
https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg00657.html
which is also depending of another from Cédric, so I'll just extract it
and send directly, sorry for the delay.

Regards,

Phil.

Re: [Qemu-devel] [PATCH 14/14] target/ppc: improve VSX_FMADD with new GEN_VSX_HELPER_VSX_MADD macro

[Richard Henderson]

On 5/5/19 3:20 AM, Mark Cave-Ayland wrote:
>> The afrm argument is no longer used.
>> This also means that e.g.
>>
>> VSX_MADD(xsmaddadp, 1, float64, VsrD(0), MADD_FLGS, 1, 1, 0)
>> VSX_MADD(xsmaddmdp, 1, float64, VsrD(0), MADD_FLGS, 0, 1, 0)
>>
>> are redundant.  Similarly with all of the other pairs.
> 
> Agreed. What do you think is the best solution here - maybe a double macro 
> that looks
> something like this?
> 
> #define VSX_MADD(op, prec, nels, tp, fld, maddflgs, sfprf, r2sp)
> _VSX_MADD(op##aprec, nels, tp, fld, maddflgs, sfprf, r2sp)
> _VSX_MADD(op##mprec, nels, tp, fld, maddflgs, sfprf, r2sp)
> 
> VSX_MADD(xsmadd, dp, 1, float64, VsrD(0), MADD_FLGS, 1, 0)

I have no idea what you're suggesting.

I am suggesting one function, xsmadddp, replacing xsmaddadp + xsmaddmdp, that
is used by both instructions.


r~

Re: [Qemu-devel] [PATCH] tests/docker: Test more components on the Fedora default image

[Alex Bennée]

Philippe Mathieu-Daudé  writes:

> Install optional dependencies of QEMU to get better coverage.
>
> The following components are now enabled:
>
>   $ ./configure
>   ...
>   Multipath support yes
>   VNC SASL support  yes
>   RDMA support  yes
>   PVRDMA supportyes
>   libiscsi support  yes
>   seccomp support   yes
>   libpmem support   yes
>   libudev   yes
>
> Note: The udev-devel package is provided by systemd-devel.
>
> Signed-off-by: Philippe Mathieu-Daudé 

Queued to testing/next, thanks.

> ---
>  tests/docker/dockerfiles/fedora.docker | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/tests/docker/dockerfiles/fedora.docker 
> b/tests/docker/dockerfiles/fedora.docker
> index 69d4a7f5d75..afbba29adaa 100644
> --- a/tests/docker/dockerfiles/fedora.docker
> +++ b/tests/docker/dockerfiles/fedora.docker
> @@ -8,6 +8,7 @@ ENV PACKAGES \
>  bzip2-devel \
>  ccache \
>  clang \
> +cyrus-sasl-devel \
>  device-mapper-multipath-devel \
>  findutils \
>  flex \
> @@ -23,13 +24,17 @@ ENV PACKAGES \
>  libaio-devel \
>  libasan \
>  libattr-devel \
> +libblockdev-mpath-devel \
>  libcap-devel \
>  libcap-ng-devel \
>  libcurl-devel \
>  libfdt-devel \
> +libiscsi-devel \
>  libjpeg-devel \
> +libpmem-devel \
>  libpng-devel \
>  librbd-devel \
> +libseccomp-devel \
>  libssh2-devel \
>  libubsan \
>  libusbx-devel \
> @@ -74,10 +79,12 @@ ENV PACKAGES \
>  pixman-devel \
>  python3 \
>  PyYAML \
> +rdma-core-devel \
>  SDL2-devel \
>  snappy-devel \
>  sparse \
>  spice-server-devel \
> +systemd-devel \
>  systemtap-sdt-devel \
>  tar \
>  usbredir-devel \


--
Alex Bennée

[Qemu-devel] [Bug 1462640] Re: shmat fails on 32-to-64 setup

[Thomas Huth]

Which version of QEMU did you use here? Does it still reproduce with the
latest version of QEMU?

** Changed in: qemu
   Status: New => Incomplete

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

Title:
  shmat fails on 32-to-64 setup

Status in QEMU:
  Incomplete

Bug description:
  
  I am trying to run a guest mips32 program (user mode) on a x86_64 host. The 
program fails on a call to shmat() reproducibly. when digging into this 
problem, I could make a small guest POC that fails when compiled as i386 (-m32) 
running on a x86_64 host, but pass when compiled as 64bit. The problem has to 
do with mmap flags.

  From what I can understand, when running 32bits guests programs, qemu
  reserve the whole guest virtual space with an mmap call. That mmap
  call specifys MAP:PRIVATE flag. When shmat is called, it tries to make
  part of that region MAP_SHARED and that fails.

  As a possible fix, it looks like it is possible to first unmap the shm
  region before calling shmat.

  steps to reproduce: 
  1 - create a file shm.c with content below
  2 - compile with: gcc -m32 shm.c -o shm32
  3 - run on a x86_64 host: qemu-i386 ./shm32 
  4 - observe shmat fails, by returning ptr -1

  5- compile without -m32: : gcc shm.c -o shm64
  6 - observe it pass: qemu-x84_64 ./shm64


  #include 
  #include 
  #include 
  #include 

  int main()
  {
  struct shmid_ds shm_desc;
  int err = 0;
  int id = shmget(IPC_PRIVATE, 688128, IPC_CREAT|IPC_EXCL|0666);
  err = shmctl(id, IPC_STAT, _desc);
  const void *at = 0x7f7df38ea000;
  void* ptr = shmat(id, at, 0);
  printf( "got err %d, ptr %p\n", err, ptr );
  }

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

Re: [Qemu-devel] [PATCH 05/14] target/ppc: introduce GEN_VSX_HELPER_X2 macro to fpu_helper.c

[Richard Henderson]

On 5/5/19 2:57 AM, Mark Cave-Ayland wrote:
> For reference the culprits here is helper_xscvqpdp(). But again if you agree 
> that it
> makes sense to create separate gen/helper functions then I can remove the 
> opcode
> later on in the series.

Yes.  Or indeed in a completely separate series.


r~

Re: [Qemu-devel] [PATCH] qcow2: Fix error handling in the compression code

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190430100802.15368-1-be...@igalia.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
time make docker-test-mingw@fedora SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20190430100802.15368-1-be...@igalia.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [Bug 1583421] Re: Please provide an option to print the default hardware configuration as command-line options, to make -nodefaults easier to use

[Thomas Huth]

** Changed in: qemu
   Importance: Undecided => Wishlist

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

Title:
  Please provide an option to print the default hardware configuration
  as command-line options, to make -nodefaults easier to use

Status in QEMU:
  New

Bug description:
  For full customization of the default set of hardware qemu supports, a
  user can pass -nodefaults and then manually specify each device they
  want.  Many specific options document what they translate to in terms
  of the full configuration model; however, the defaults for any given
  platform don't.

  I'd love to have documentation of the default hardware configuration,
  in terms of qemu command-line options, to make it easy to run qemu
  -nodefaults, paste in the default command-line, and edit it.

  As this varies by emulated machine, perhaps qemu could have a command-
  line option to print a specific machine (e.g. pc-i440fx-2.5) in the
  form of qemu command-line options?

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

Re: [Qemu-devel] [PATCH 04/14] target/ppc: introduce GEN_VSX_HELPER_X3 macro to fpu_helper.c

[Richard Henderson]

On 5/5/19 2:52 AM, Mark Cave-Ayland wrote:
> Right, it looks like VSX_CMP is the culprit here. Am I right in thinking that 
> it's
> best to remove the opc parameter from GEN_VSX_HELPER_X3 above, and then have a
> separate gen and helper function for just the VSX_CMP instructions? 
> Presumably this
> reduces of the overhead at both translation and execution time for the 
> instructions
> that don't require it.

Yep.

I think the best fix for VSX_CMP is to return the value that is to be assigned
to cr[6], and let it assign like so:

  gen_helper_foo(cpu_crf[6], other, arguments);

(Or if the opcode bit is unset,

  TCGv_i32 ignored = tcg_temp_new_i32();
  gen_helper_foo(ignored, other arguments);
  tcg_temp_free_i32(ignored);
)

at which point these functions do not modify tcg globals, so the decl
can be improved to

  DEF_HELPER_FLAGS_2(xvcmpeqdp, TCG_CALL_NO_RWG, i32, ptr, ptr, ptr)

To keep the assignment vs exception order, you remove the direct call to
do_float_check_status and use gen_helper_float_check_status.


r~

[Qemu-devel] [PATCH] virtfs: Add missing "id" parameter in documentation

[Thomas Huth]

... and remove the square brackets from "path" and "security_model",
since these parameters are not optional.

Buglink: https://bugs.launchpad.net/qemu/+bug/1581976
Signed-off-by: Thomas Huth 
---
 qemu-options.hx | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 51802cbb26..9571ddd141 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1311,7 +1311,7 @@ DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
 
 STEXI
 
-@item -virtfs 
@var{fsdriver}[,path=@var{path}],mount_tag=@var{mount_tag}[,security_model=@var{security_model}][,writeout=@var{writeout}][,readonly][,socket=@var{socket}|sock_fd=@var{sock_fd}][,fmode=@var{fmode}][,dmode=@var{dmode}]
+@item -virtfs 
@var{fsdriver},path=@var{path},mount_tag=@var{mount_tag},security_model=@var{security_model}[,id=@var{id}][,writeout=@var{writeout}][,readonly][,socket=@var{socket}|sock_fd=@var{sock_fd}][,fmode=@var{fmode}][,dmode=@var{dmode}]
 @findex -virtfs
 
 The general form of a Virtual File system pass-through options are:
-- 
2.21.0

Re: [Qemu-devel] [PATCH 02/14] target/ppc: remove getVSR()/putVSR() from mem_helper.c

[Richard Henderson]

On 5/5/19 2:34 AM, Mark Cave-Ayland wrote:
>>>  EA = tcg_temp_new();\
>>> -xt = tcg_const_tl(xT(ctx->opcode)); \
>>>  gen_set_access_type(ctx, ACCESS_INT);   \
>>>  gen_addr_register(ctx, EA); \
>>> -gen_helper_##name(cpu_env, EA, xt, cpu_gpr[rB(ctx->opcode)]); \
>>> +xt = tcg_const_tl(xT(ctx->opcode)); \
>>> +rb = tcg_const_tl(rB(ctx->opcode)); \
>>> +gen_helper_##name(cpu_env, EA, xt, rb); \
>>>  tcg_temp_free(EA);  \
>>>  tcg_temp_free(xt);  \
>>> +tcg_temp_free(rb);  \
>>>  }
>>
>> Why are you adjusting the function to pass the rB register number rather than
>> the contents of rB?  That seems the wrong way around...
> 
> I think what I was trying to do here was eliminate the cpu_gpr since it
> feels to me that with the vector patchsets and your negative offset patches
> that this should be the way to go for accessing CPUState rather than using
> TCG globals.

Not for the integer register set.

> Looking at this again I realise the solution is really the same as is
> currently used
> for gen_load_spr() so I can use something like this:>
> static inline void gen_load_gpr(TCGv t, int reg)
> {
> tcg_gen_ld_tl(t, cpu_env, offsetof(CPUPPCState, gpr[reg]));
> }
> 
> Does this seem reasonable as a solution?

No, this will fail quickly.


r~

Re: [Qemu-devel] [PATCH 01/14] target/ppc: remove getVSR()/putVSR() from fpu_helper.c

[Richard Henderson]

On 5/5/19 2:27 AM, Mark Cave-Ayland wrote:
> I've spent a bit of time today going through the functions and it seems that 
> all
> functions which have an xt parameter, minus a couple of the TEST macros, 
> require the
> result to be calculated in a local variable first.
> 
> I think the best solution is still to remove getVSR()/putVSR() but replace 
> them with
> macros for copying and zeroing like this:
> 
> #define VSRCPY(d, s) (memcpy(d, s, sizeof(ppc_vsr_t)))
> #define VSRZERO(d)   (memset(d, 0, sizeof(ppc_vsr_t)))

Local variable, yes.  But I see no reason for macros.

  ppc_vsr_t res = { };
  ...
  *xt = res;


r~

[Qemu-devel] [PULL 23/28] hw/arm: Express dependencies of the remaining IMX boards with Kconfig

[Thomas Huth]

IMX25, IMX7 and IMX6UL were still missing the Kconfig dependencies.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak |  2 --
 hw/arm/Kconfig  | 19 +++
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 1455d083d8..6dc388c43e 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -37,7 +37,6 @@ CONFIG_SABRELITE=y
 CONFIG_EMCRAFT_SF2=y
 
 CONFIG_VGA=y
-CONFIG_IMX_FEC=y
 
 CONFIG_NRF51_SOC=y
 
@@ -49,4 +48,3 @@ CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
-CONFIG_PCI_EXPRESS_DESIGNWARE=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7af94a8184..4a14749792 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -285,6 +285,10 @@ config XLNX_VERSAL
 
 config FSL_IMX25
 bool
+select IMX
+select IMX_FEC
+select IMX_I2C
+select DS1338
 
 config FSL_IMX31
 bool
@@ -299,6 +303,7 @@ config FSL_IMX6
 select IMX
 select IMX_FEC
 select IMX_I2C
+select SDHCI
 
 config ASPEED_SOC
 bool
@@ -324,12 +329,26 @@ config MPS2
 
 config FSL_IMX7
 bool
+imply PCI_DEVICES
+imply TEST_DEVICES
+select A15MPCORE
+select PCI
+select IMX
+select IMX_FEC
+select IMX_I2C
+select PCI_EXPRESS_DESIGNWARE
+select SDHCI
 
 config ARM_SMMUV3
 bool
 
 config FSL_IMX6UL
 bool
+select A15MPCORE
+select IMX
+select IMX_FEC
+select IMX_I2C
+select SDHCI
 
 config NRF51_SOC
 bool
-- 
2.21.0

[Qemu-devel] [PULL 27/28] hw/arm: Express dependencies of the musca machines with Kconfig

[Thomas Huth]

Dependencies have been determined with trial-and-error and by
looking at the musca.c source file.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 hw/arm/Kconfig | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 9609caef87..af8cffde9c 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -75,6 +75,12 @@ config MAINSTONE
 select PFLASH_CFI01
 select SMC91C111
 
+config MUSCA
+bool
+select ARMSSE
+select PL011
+select PL031
+
 config MUSICPAL
 bool
 select BITBANG_I2C
@@ -427,6 +433,3 @@ config ARMSSE_CPUID
 
 config ARMSSE_MHU
 bool
-
-config MUSCA
-bool
-- 
2.21.0

[Qemu-devel] [PULL 25/28] hw/arm: Express dependencies of the ZynqMP zcu102 machine with Kconfig

[Thomas Huth]

This cleans up most settings in default-configs/aarch64-softmmu.mak.

Reviewed-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 default-configs/aarch64-softmmu.mak |  4 
 hw/arm/Kconfig  | 11 +++
 hw/display/Kconfig  |  1 +
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/default-configs/aarch64-softmmu.mak 
b/default-configs/aarch64-softmmu.mak
index 313490fb38..49ff415ee4 100644
--- a/default-configs/aarch64-softmmu.mak
+++ b/default-configs/aarch64-softmmu.mak
@@ -3,9 +3,5 @@
 # We support all the 32 bit boards so need all their config
 include arm-softmmu.mak
 
-CONFIG_AUX=y
-CONFIG_DDC=y
-CONFIG_DPCD=y
-CONFIG_XLNX_ZYNQMP=y
 CONFIG_XLNX_ZYNQMP_ARM=y
 CONFIG_XLNX_VERSAL=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index da091ac217..eff61efab9 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -279,6 +279,17 @@ config STM32F205_SOC
 
 config XLNX_ZYNQMP_ARM
 bool
+select AHCI
+select ARM_GIC
+select CADENCE
+select DDC
+select DPCD
+select SDHCI
+select SSI
+select SSI_M25P80
+select XILINX_AXI
+select XILINX_SPIPS
+select XLNX_ZYNQMP
 
 config XLNX_VERSAL
 bool
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 0577e68c8e..bb95f8d6a4 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -108,6 +108,7 @@ config VIRTIO_VGA
 
 config DPCD
 bool
+select AUX
 
 config ATI_VGA
 bool
-- 
2.21.0

[Qemu-devel] [PULL 28/28] hw/arm: Remove hard-enablement of the remaining PCI devices

[Thomas Huth]

The PCI devices should be pulled in by default if PCI_DEVICES
is set, so there is no need anymore to enforce them in the configs
file.

Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 8 
 1 file changed, 8 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 233937f394..f23ecfd5c5 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -36,14 +36,6 @@ CONFIG_DIGIC=y
 CONFIG_SABRELITE=y
 CONFIG_EMCRAFT_SF2=y
 CONFIG_MICROBIT=y
-
-CONFIG_VGA=y
-
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
 CONFIG_FSL_IMX6UL=y
-
-CONFIG_PCIE_PORT=y
-CONFIG_XIO3130=y
-CONFIG_IOH3420=y
-CONFIG_I82801B11=y
-- 
2.21.0

[Qemu-devel] [PULL 24/28] hw/arm: Express dependencies of the microbit / nrf51 machine with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the NRF51 / microbit machine.

Reviewed-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 3 +--
 hw/arm/Kconfig  | 6 ++
 hw/arm/Makefile.objs| 3 ++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 6dc388c43e..233937f394 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -35,11 +35,10 @@ CONFIG_RASPI=y
 CONFIG_DIGIC=y
 CONFIG_SABRELITE=y
 CONFIG_EMCRAFT_SF2=y
+CONFIG_MICROBIT=y
 
 CONFIG_VGA=y
 
-CONFIG_NRF51_SOC=y
-
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
 CONFIG_FSL_IMX6UL=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 4a14749792..da091ac217 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -350,8 +350,14 @@ config FSL_IMX6UL
 select IMX_I2C
 select SDHCI
 
+config MICROBIT
+bool
+select NRF51_SOC
+
 config NRF51_SOC
 bool
+select I2C
+select ARM_V7M
 
 config EMCRAFT_SF2
 bool
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index eae9f6c442..994e67dd0d 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -8,6 +8,7 @@ obj-$(CONFIG_EMCRAFT_SF2) += msf2-som.o
 obj-$(CONFIG_HIGHBANK) += highbank.o
 obj-$(CONFIG_INTEGRATOR) += integratorcp.o
 obj-$(CONFIG_MAINSTONE) += mainstone.o
+obj-$(CONFIG_MICROBIT) += microbit.o
 obj-$(CONFIG_MUSICPAL) += musicpal.o
 obj-$(CONFIG_NETDUINO2) += netduino2.o
 obj-$(CONFIG_NSERIES) += nseries.o
@@ -48,4 +49,4 @@ obj-$(CONFIG_ARMSSE) += armsse.o
 obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
 obj-$(CONFIG_ARM_SMMUV3) += smmu-common.o smmuv3.o
 obj-$(CONFIG_FSL_IMX6UL) += fsl-imx6ul.o mcimx6ul-evk.o
-obj-$(CONFIG_NRF51_SOC) += nrf51_soc.o microbit.o
+obj-$(CONFIG_NRF51_SOC) += nrf51_soc.o
-- 
2.21.0

[Qemu-devel] [PULL 22/28] hw/arm: Express dependencies of the MSF2 / EMCRAFT_SF2 machine with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the emcraft-sf2 machine - we also
distinguish between the machine (CONFIG_EMCRAFT_SF2) and the SoC
(CONFIG_MSF2) now.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 3 +--
 hw/arm/Kconfig  | 8 
 hw/arm/Makefile.objs| 3 ++-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index ef7dd7156a..1455d083d8 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -34,9 +34,9 @@ CONFIG_MPS2=y
 CONFIG_RASPI=y
 CONFIG_DIGIC=y
 CONFIG_SABRELITE=y
+CONFIG_EMCRAFT_SF2=y
 
 CONFIG_VGA=y
-CONFIG_SSI_M25P80=y
 CONFIG_IMX_FEC=y
 
 CONFIG_NRF51_SOC=y
@@ -49,5 +49,4 @@ CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
 CONFIG_I82801B11=y
-CONFIG_MSF2=y
 CONFIG_PCI_EXPRESS_DESIGNWARE=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 0a1eab568b..7af94a8184 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -334,9 +334,17 @@ config FSL_IMX6UL
 config NRF51_SOC
 bool
 
+config EMCRAFT_SF2
+bool
+select MSF2
+select SSI_M25P80
+
 config MSF2
 bool
+select ARM_V7M
 select PTIMER
+select SERIAL
+select SSI
 
 config ZAURUS
 bool
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index fadd69882c..eae9f6c442 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -4,6 +4,7 @@ obj-$(CONFIG_ARM_VIRT) += virt.o
 obj-$(CONFIG_ACPI) += virt-acpi-build.o
 obj-$(CONFIG_DIGIC) += digic_boards.o
 obj-$(CONFIG_EXYNOS4) += exynos4_boards.o
+obj-$(CONFIG_EMCRAFT_SF2) += msf2-som.o
 obj-$(CONFIG_HIGHBANK) += highbank.o
 obj-$(CONFIG_INTEGRATOR) += integratorcp.o
 obj-$(CONFIG_MAINSTONE) += mainstone.o
@@ -41,7 +42,7 @@ obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
 obj-$(CONFIG_MPS2) += mps2.o
 obj-$(CONFIG_MPS2) += mps2-tz.o
-obj-$(CONFIG_MSF2) += msf2-soc.o msf2-som.o
+obj-$(CONFIG_MSF2) += msf2-soc.o
 obj-$(CONFIG_MUSCA) += musca.o
 obj-$(CONFIG_ARMSSE) += armsse.o
 obj-$(CONFIG_FSL_IMX7) += fsl-imx7.o mcimx7d-sabre.o
-- 
2.21.0

[Qemu-devel] [PULL 26/28] hw/arm: Express dependencies of the xlnx-versal-virt machine with Kconfig

[Thomas Huth]

Dependencies have been determined with trial-and-error and by
looking at the xlnx-versal.c source file.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 hw/arm/Kconfig | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index eff61efab9..9609caef87 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -293,6 +293,10 @@ config XLNX_ZYNQMP_ARM
 
 config XLNX_VERSAL
 bool
+select ARM_GIC
+select PL011
+select CADENCE
+select VIRTIO_MMIO
 
 config FSL_IMX25
 bool
-- 
2.21.0

[Qemu-devel] [PULL 21/28] hw/arm: Express dependencies of sabrelite with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the Sabrelite / iMX6 machine.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 4 +---
 hw/arm/Kconfig  | 9 +
 hw/arm/Makefile.objs| 3 ++-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 76508e3910..ef7dd7156a 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -33,6 +33,7 @@ CONFIG_NETDUINO2=y
 CONFIG_MPS2=y
 CONFIG_RASPI=y
 CONFIG_DIGIC=y
+CONFIG_SABRELITE=y
 
 CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
@@ -40,13 +41,10 @@ CONFIG_IMX_FEC=y
 
 CONFIG_NRF51_SOC=y
 
-CONFIG_FSL_IMX6=y
 CONFIG_FSL_IMX25=y
 CONFIG_FSL_IMX7=y
 CONFIG_FSL_IMX6UL=y
 
-CONFIG_IMX_I2C=y
-
 CONFIG_PCIE_PORT=y
 CONFIG_XIO3130=y
 CONFIG_IOH3420=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index abf2af0967..0a1eab568b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -177,6 +177,11 @@ config REALVIEW
 select DS1338 # I2C RTC+NVRAM
 select USB_OHCI
 
+config SABRELITE
+bool
+select FSL_IMX6
+select SSI_M25P80
+
 config STELLARIS
 bool
 select ARM_V7M
@@ -290,6 +295,10 @@ config FSL_IMX31
 
 config FSL_IMX6
 bool
+select A9MPCORE
+select IMX
+select IMX_FEC
+select IMX_I2C
 
 config ASPEED_SOC
 bool
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 4f591ca487..fadd69882c 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -22,6 +22,7 @@ obj-$(CONFIG_COLLIE) += collie.o
 obj-$(CONFIG_VERSATILE) += versatilepb.o
 obj-$(CONFIG_VEXPRESS) += vexpress.o
 obj-$(CONFIG_ZYNQ) += xilinx_zynq.o
+obj-$(CONFIG_SABRELITE) += sabrelite.o
 
 obj-$(CONFIG_ARM_V7M) += armv7m.o
 obj-$(CONFIG_EXYNOS4) += exynos4210.o
@@ -36,7 +37,7 @@ obj-$(CONFIG_XLNX_ZYNQMP_ARM) += xlnx-zynqmp.o xlnx-zcu102.o
 obj-$(CONFIG_XLNX_VERSAL) += xlnx-versal.o xlnx-versal-virt.o
 obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
 obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
-obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
+obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
 obj-$(CONFIG_MPS2) += mps2.o
 obj-$(CONFIG_MPS2) += mps2-tz.o
-- 
2.21.0

[Qemu-devel] [PULL 20/28] hw/arm: Express dependencies of canon-a1100 with Kconfig

[Thomas Huth]

Add Kconfig dependencies for the DIGIC / canon-a1100 machine.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 default-configs/arm-softmmu.mak | 2 +-
 hw/arm/Kconfig  | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 22bff20b32..76508e3910 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -32,12 +32,12 @@ CONFIG_ASPEED_SOC=y
 CONFIG_NETDUINO2=y
 CONFIG_MPS2=y
 CONFIG_RASPI=y
+CONFIG_DIGIC=y
 
 CONFIG_VGA=y
 CONFIG_SSI_M25P80=y
 CONFIG_IMX_FEC=y
 
-CONFIG_DIGIC=y
 CONFIG_NRF51_SOC=y
 
 CONFIG_FSL_IMX6=y
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 40be78303c..abf2af0967 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -32,6 +32,7 @@ config CUBIEBOARD
 config DIGIC
 bool
 select PTIMER
+select PFLASH_CFI02
 
 config EXYNOS4
 bool
-- 
2.21.0