Re: [PATCH] hw/ppc/vof: Add missing includes

2022-01-30 Thread Cédric Le Goater

On 1/30/22 20:38, Philippe Mathieu-Daudé wrote:

Cc'ing qemu-trivial@



It is queued in ppc-7.0. I should send a PR today or tomorrow.

Thanks,

C.


On 22/1/22 01:31, Philippe Mathieu-Daudé wrote:

vof.h requires "qom/object.h" for DECLARE_CLASS_CHECKERS(),
"exec/memory.h" for address_space_read/write(),
"exec/address-spaces.h" for address_space_memory
and more importantly "cpu.h" for target_ulong.

vof.c doesn't need "exec/ram_addr.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/ppc/vof.c | 1 -
  include/hw/ppc/vof.h | 5 +
  2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index 73adc44ec2..2b63a62875 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -16,7 +16,6 @@
  #include "qemu/units.h"
  #include "qemu/log.h"
  #include "qapi/error.h"
-#include "exec/ram_addr.h"
  #include "exec/address-spaces.h"
  #include "hw/ppc/vof.h"
  #include "hw/ppc/fdt.h"
diff --git a/include/hw/ppc/vof.h b/include/hw/ppc/vof.h
index 97fdef758b..f8c0effcaf 100644
--- a/include/hw/ppc/vof.h
+++ b/include/hw/ppc/vof.h
@@ -6,6 +6,11 @@
  #ifndef HW_VOF_H
  #define HW_VOF_H
+#include "qom/object.h"
+#include "exec/address-spaces.h"
+#include "exec/memory.h"
+#include "cpu.h"
+
  typedef struct Vof {
  uint64_t top_addr; /* copied from rma_size */
  GArray *claimed; /* array of SpaprOfClaimed */







Re: build-oss-fuzz CI job often times out

2022-01-30 Thread Thomas Huth

On 29/1/22 14:34, Peter Maydell wrote:

Hi; the build-oss-fuzz gitlab CI job seems to intermittently
but quite commonly hit the 1 hour timeout mark and get killed.
Examples from the last couple of days:

https://gitlab.com/qemu-project/qemu/-/jobs/2030815488
https://gitlab.com/qemu-project/qemu/-/jobs/2029246068
https://gitlab.com/qemu-project/qemu/-/jobs/2029013479
https://gitlab.com/qemu-project/qemu/-/jobs/2024871970
https://gitlab.com/qemu-project/qemu/-/jobs/2022584981

Can we do anything to cut down on the runtime of this job
and make it reliably complete inside the time limit?


All the jobs that you've listed hang in the very same test 
(qtest-i386/boot-serial-test), so I assume it's rather a test that now hangs 
occasionally, and not a generic slowness (otherwise the jobs would fail 
sometimes earlier, sometimes later).


Thus we likely have a regression in the code that only shows up occasionally 
in these builds... Can you mark a point in time when these issues first 
happened?


 Thomas




Re: [PATCH 10/16] Revert "Revert "arm: tcg: Adhere to SMCCC 1.3 section 5.2""

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

Now that we have arranged for all the affected board models
to not enable the PSCI emulation if they are running guest code
at EL3, we can revert commit 4825eaae4fdd56fba0f, thus
reinstating commit 9fcd15b9193e819b, without bringing back the
regressions that caused us to revert it.

For clarity, here is the original commit message of 9fcd15b9193e819b:

The SMCCC 1.3 spec section 5.2 says

   The Unknown SMC Function Identifier is a sign-extended value of (-1)
   that is returned in the R0, W0 or X0 registers. An implementation must
   return this error code when it receives:

 * An SMC or HVC call with an unknown Function Identifier
 * An SMC or HVC call for a removed Function Identifier
 * An SMC64/HVC64 call from AArch32 state

To comply with these statements, let's always return -1 when we encounter
an unknown HVC or SMC call.

Signed-off-by: Peter Maydell
---
  target/arm/psci.c | 35 ++-
  1 file changed, 6 insertions(+), 29 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH] tests/9pfs: Use g_autofree and g_autoptr where possible

2022-01-30 Thread Greg Kurz
On Sat, 29 Jan 2022 13:33:59 +0100
Christian Schoenebeck  wrote:

> On Freitag, 28. Januar 2022 12:49:58 CET Christian Schoenebeck wrote:
> > On Mittwoch, 26. Januar 2022 18:11:36 CET Greg Kurz wrote:
> > > The template pointer in virtio_9p_create_local_test_dir() is leaked.
> > > Add the g_autofree annotation to fix that. While here, convert the
> > > rest of the virtio 9p test code to using g_autofree or g_autoptr
> > > where possible, since this is the preferred approach to avoid potential
> > > leaks in the future.
> > > 
> > > Based-on:
> > >  > > e
> > > .com> Signed-off-by: Greg Kurz 
> > > ---
> > > 
> > >  tests/qtest/libqos/virtio-9p.c | 15 +--
> > >  1 file changed, 5 insertions(+), 10 deletions(-)
> > 
> > I fear there is something wrong with this patch:
> > 
> > # Start of local tests
> > # starting QEMU: exec x86_64-softmmu/qemu-system-x86_64 -qtest
> > unix:/tmp/qtest-4234.sock -qtest-log /dev/null -chardev
> > socket,path=/tmp/qtest-4234.qmp,id=char0 -mon chardev=char0,mode=control
> > -display none -M pc  -fsdev
> > local,id=fsdev0,path='',security_model=mapped-xattr -device
> > virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest
> > qemu-system-x86_64: -device
> > virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest: cannot initialize
> > fsdev 'fsdev0': failed to open '': No such file or directory Broken pipe
> > Aborted
> 
> Reason ...
> 
> > > diff --git a/tests/qtest/libqos/virtio-9p.c
> > > b/tests/qtest/libqos/virtio-9p.c index ef96ef006adc..0a0d0d16709b 100644
> > > --- a/tests/qtest/libqos/virtio-9p.c
> > > +++ b/tests/qtest/libqos/virtio-9p.c
> > > @@ -40,14 +40,13 @@ static char *concat_path(const char* a, const char* b)
> > > 
> > >  void virtio_9p_create_local_test_dir(void)
> > >  {
> > >  
> > >  struct stat st;
> > > 
> > > -char *pwd = g_get_current_dir();
> > > -char *template = concat_path(pwd, "qtest-9p-local-XX");
> > > +g_autofree char *pwd = g_get_current_dir();
> > > +g_autofree char *template = concat_path(pwd,
> > > "qtest-9p-local-XX");
> > > 
> > >  local_test_path = mkdtemp(template);
> 
> ... mkdtemp() does not allocate a new buffer, it just modifies the character 
> array passed, i.e. the address returned by mkdtemp() equals the address of 
> variable 'template', and when virtio_9p_create_local_test_dir() scope is 
> left, 
> the global variable 'local_test_path' would then point to freed memory.
> 

I hate global variables ;-) and the 'Returned result must be freed' comment
in 'concat_path()' is slightly misleading in this respect.

> I would drop g_autofree from template:
> 
> char *template = concat_path(pwd, "qtest-9p-local-XX");
> 
> And if it helps to silence a leak warning (haven't tested), to prepend 
> g_autofree to the global variable instead:
> 
> static g_autofree char *local_test_path;
> 

The way to go is either drop the g_autofree annotation as you're
suggesting, but this would make the comment in 'concat_path()'
even more awkward, or go forward with the glib way and use
g_steal_pointer() which maps exactly to what the code is doing.

> Best regards,
> Christian Schoenebeck
> 
> 




Re: [PATCH v6 0/7] tests: Refresh lcitool submodule & remove libxml2

2022-01-30 Thread Alessandro Di Federico via
On Tue, 25 Jan 2022 11:59:38 +0100
Philippe Mathieu-Daudé via  wrote:

> I'm seeing the same issue with these domains since mid december:
> 
> ...
> - rev.ng
> 
> ...
> https://lore.kernel.org/qemu-devel/20220105185720.0d4fc159@orange/
> ...

I've tried to look into this and it looks like our set up should be OK.
We enabled SPF (i.e., a rule stating that only our mailserver can send
e-mail with our domain in "From:") and DKIM (i.e., our mailserver signs
certain portions of the e-mail). We also enabled DMARC which
coordinates the two.

Now, as far as I understand, mailing lists can either rewrite the
"From" header (as qemu-devel does) or leave it as it is. In the latter
situation, SPF will fail but DMARC should instruct MTAs to check
DKIM, and that should pass.

https://begriffs.com/posts/2018-09-18-dmarc-mailing-list.html

https://dmarc.org/wiki/FAQ#I_operate_a_mailing_list_and_I_want_to_interoperate_with_DMARC.2C_what_should_I_do.3F

DKIM signature can be corrupted in case the mailing list tampers with
the subject or the body of the e-mail, but this doesn't seem to be the
case: I've tried to manually verify the DKIM signature of the same
e-mail that I got both from the mailing list and directly from the
sender (I was in Cc), and they both verify correctly.

tl;dr I *think* rewriting the From header should not be necessary for
our domain.

If you guys think this is not the case and there's something we can do
to improve the situation (other than adding gmail.com to our SPF
record), let me know.

-- 
Alessandro Di Federico
rev.ng Labs



Re: [PATCH 07/16] hw/arm/versal: Let boot.c handle PSCI enablement

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

Instead of setting the CPU psci-conduit and start-powered-off
properties in the xlnx-versal-virt board code, set the arm_boot_info
psci_conduit field so that the boot.c code can do it.

This will fix a corner case where we were incorrectly enabling PSCI
emulation when booting guest code into EL3 because it was an ELF file
passed to -kernel.  (EL3 guest code started via -bios, -pflash, or
the generic loader was already being run with PSCI emulation
disabled.)

Note that EL3 guest code has no way to turn on the secondary CPUs
because there's no emulated power controller, but this was already
true for EL3 guest code run via -bios, -pflash, or the generic
loader.

Signed-off-by: Peter Maydell
---
  include/hw/arm/xlnx-versal.h | 1 -
  hw/arm/xlnx-versal-virt.c| 6 --
  hw/arm/xlnx-versal.c | 5 +
  3 files changed, 5 insertions(+), 7 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 09/16] hw/arm: highbank: For EL3 guests, don't enable PSCI, start all cores

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

Change the highbank/midway boards to use the new boot.c functionality
to allow us to enable psci-conduit only if the guest is being booted
in EL1 or EL2, so that if the user runs guest EL3 firmware code our
PSCI emulation doesn't get in its way.

To do this we stop setting the psci-conduit and start-powered-off
properties on the CPU objects in the board code, and instead set the
psci_conduit field in the arm_boot_info struct to tell the common
boot loader code that we'd like PSCI if the guest is starting at an
EL that it makes sense with (in which case it will set these
properties).

This means that when running guest code at EL3, all the cores
will start execution at once on poweron. This matches the
real hardware behaviour. (A brief description of the hardware
boot process is in the u-boot documentation for these boards:
https://u-boot.readthedocs.io/en/latest/board/highbank/highbank.html#boot-process
  -- in theory one might run the 'a9boot'/'a15boot' secure monitor
code in QEMU, though we probably don't emulate enough for that.)

This affects the highbank and midway boards.

Signed-off-by: Peter Maydell
---
  hw/arm/highbank.c | 7 +--
  1 file changed, 1 insertion(+), 6 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 08/16] hw/arm/virt: Let boot.c handle PSCI enablement

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

Instead of setting the CPU psci-conduit and start-powered-off
properties in the virt board code, set the arm_boot_info psci_conduit
field so that the boot.c code can do it.

This will fix a corner case where we were incorrectly enabling PSCI
emulation when booting guest code into EL3 because it was an ELF file
passed to -kernel or to the generic loader.  (EL3 guest code started
via -bios or -pflash was already being run with PSCI emulation
disabled.)

Signed-off-by: Peter Maydell 
---
  hw/arm/virt.c | 12 +---
  1 file changed, 1 insertion(+), 11 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH 06/16] hw/arm/xlnx-zcu102: Don't enable PSCI conduit when booting guest in EL3

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

Change the Xilinx ZynqMP-based board xlnx-zcu102 to use the new
boot.c functionality to allow us to enable psci-conduit only if
the guest is being booted in EL1 or EL2, so that if the user runs
guest EL3 firmware code our PSCI emulation doesn't get in its
way.

To do this we stop setting the psci-conduit property on the CPU
objects in the SoC code, and instead set the psci_conduit field in
the arm_boot_info struct to tell the common boot loader code that
we'd like PSCI if the guest is starting at an EL that it makes
sense with.

Note that this means that EL3 guest code will have no way
to power on secondary cores, because we don't model any
kind of power controller that does that on this SoC.

Signed-off-by: Peter Maydell
---
Again, if anybody knows the real-hardware EL3 behaviour for
CPUs that would be great.
---
  hw/arm/xlnx-zcu102.c |  1 +
  hw/arm/xlnx-zynqmp.c | 13 -
  2 files changed, 9 insertions(+), 5 deletions(-)


Acked-by: Richard Henderson 

r~



Re: [PATCH 04/16] hw/arm: imx: Don't enable PSCI conduit when booting guest in EL3

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

Change the iMX-SoC based boards to use the new boot.c functionality
to allow us to enable psci-conduit only if the guest is being booted
in EL1 or EL2, so that if the user runs guest EL3 firmware code our
PSCI emulation doesn't get in its way.

To do this we stop setting the psci-conduit property on the CPU
objects in the SoC code, and instead set the psci_conduit field in
the arm_boot_info struct to tell the common boot loader code that
we'd like PSCI if the guest is starting at an EL that it makes
sense with.

This affects the mcimx6ul-evk and mcimx7d-sabre boards.

Note that for the mcimx7d board, this means that when running guest
code at EL3 there is currently no way to power on the secondary CPUs,
because we do not currently have a model of the system reset
controller module which should be used to do that for the imx7 SoC,
only for the imx6 SoC.  (Previously EL3 code which knew it was
running on QEMU could use a PSCI call to do this.) This doesn't
affect the imx6ul-evk board because it is uniprocessor.

Signed-off-by: Peter Maydell
---
I don't have the i.mx7 manual to hand, so I'm partly making
assumptions based on the i.mx6 behaviour. If somebody with the
manual could double-check that it does indeed start up with the
secondary CPUs powered down via the SRC that would be great.
---
  hw/arm/fsl-imx6ul.c| 2 --
  hw/arm/fsl-imx7.c  | 8 
  hw/arm/mcimx6ul-evk.c  | 1 +
  hw/arm/mcimx7d-sabre.c | 1 +
  4 files changed, 6 insertions(+), 6 deletions(-)


Acked-by: Richard Henderson 

r~



Re: [RFC 1/5] target/riscv: Add the privileged spec version 1.12.0

2022-01-30 Thread Alistair Francis
On Sat, Jan 29, 2022 at 10:57 AM Atish Kumar Patra  wrote:
>
>
>
> On Sun, Jan 23, 2022 at 11:59 PM Richard Henderson 
>  wrote:
>>
>> On 1/21/22 7:07 AM, Atish Patra wrote:
>> > Add the definition for ratified privileged specification version v1.12
>> >
>> > Signed-off-by: Atish Patra 
>> > ---
>> >   target/riscv/cpu.h | 1 +
>> >   1 file changed, 1 insertion(+)
>> >
>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > index 4d630867650a..671f65100b1a 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -82,6 +82,7 @@ enum {
>> >
>> >   #define PRIV_VERSION_1_10_0 0x00011000
>> >   #define PRIV_VERSION_1_11_0 0x00011100
>> > +#define PRIV_VERSION_1_12_0 0x00011200
>>
>> Is there any good reason for defining things this way, as opposed to a 
>> simple enumeration?
>> A simple enum would eliminate the need for
>>
>
> Agreed. A simple enum would be much nicer. I was just following the previous 
> definition of
> PRIV_VERSION_1_10_0 & PRIV_VERSION_1_11_0.
>
> I am not sure about the reason behind this scheme.
>
> @Alistair Francis Is there any history behind this scheme ?

I don't think so

> or Are you okay if I change it ?

Yep :)

Alistair



Re: [PATCH 3/4] acpi: fix OEM ID/OEM Table ID padding

2022-01-30 Thread Ani Sinha
On Wed, Jan 12, 2022 at 6:33 PM Igor Mammedov  wrote:
>
> Commit [2] broke original '\0' padding of OEM ID and OEM Table ID
> fields in headers of ACPI tables. While it doesn't have impact on
> default values since QEMU uses 6 and 8 characters long values
> respectively, it broke usecase where IDs are provided on QEMU CLI.
> It shouldn't affect guest (but may cause licensing verification
> issues in guest OS).
> One of the broken usecases is user supplied SLIC table with IDs
> shorter than max possible length, where [2] mangles IDs with extra
> spaces in RSDT and FADT tables whereas guest OS expects those to
> mirror the respective values of the used SLIC table.
>
> Fix it by replacing whitespace padding with '\0' padding in
> accordance with [1] and expectations of guest OS
>
> 1) ACPI spec, v2.0b
>17.2 AML Grammar Definition
>...
>//OEM ID of up to 6 characters. If the OEM ID is
>//shorter than 6 characters, it can be terminated
>//with a NULL character.

On the other hand, from
https://uefi.org/specs/ACPI/6.4/21_ACPI_Data_Tables_and_Table_Def_Language/ACPI_Data_Tables.html
,

"For example, the OEM ID and OEM Table ID in the common ACPI table
header (shown above) are fixed at six and eight characters,
respectively. They are not necessarily null terminated"

I also checked version 5 and the verbiage is the same. I think not
terminating with a null is not incorrect.

>
> 2)
> Fixes: 602b458201 ("acpi: Permit OEM ID and OEM table ID fields to be 
> changed")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/707
> Reported-by: Dmitry V. Orekhov 
> Signed-off-by: Igor Mammedov 
> Cc: qemu-sta...@nongnu.org
> ---
>  hw/acpi/aml-build.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index b3b3310df3..65148d5b9d 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1724,9 +1724,9 @@ void acpi_table_begin(AcpiTable *desc, GArray *array)
>  build_append_int_noprefix(array, 0, 4); /* Length */
>  build_append_int_noprefix(array, desc->rev, 1); /* Revision */
>  build_append_int_noprefix(array, 0, 1); /* Checksum */
> -build_append_padded_str(array, desc->oem_id, 6, ' '); /* OEMID */
> +build_append_padded_str(array, desc->oem_id, 6, '\0'); /* OEMID */
>  /* OEM Table ID */
> -build_append_padded_str(array, desc->oem_table_id, 8, ' ');
> +build_append_padded_str(array, desc->oem_table_id, 8, '\0');
>  build_append_int_noprefix(array, 1, 4); /* OEM Revision */
>  g_array_append_vals(array, ACPI_BUILD_APPNAME8, 4); /* Creator ID */
>  build_append_int_noprefix(array, 1, 4); /* Creator Revision */
> --
> 2.31.1
>



[FAQ] how to build and play vhost-user-blk with qemu code

2022-01-30 Thread Gao, Wayne
Hello dear experts.

I would like to try vhost-user-blk, may I know what is the guide for this?
For now I do not know how to build and run it, and how to start one vhost 
target emulator?

https://github.com/qemu/qemu/tree/master/contrib/vhost-user-blk


[PATCH v4 7/7] target/riscv: add a MAINTAINERS entry for XVentanaCondOps

2022-01-30 Thread Philipp Tomsich
The XVentanaCondOps extension is supported by VRULL on behalf of the
Ventana Micro.  Add myself as a point-of-contact.

Signed-off-by: Philipp Tomsich 
Reviewed-by: Richard Henderson 

---

(no changes since v3)

Changes in v3:
- add a MAINTAINERS entry for XVentanaCondOps

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

diff --git a/MAINTAINERS b/MAINTAINERS
index b43344fa98..2e0b2ae947 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -286,6 +286,13 @@ F: include/hw/riscv/
 F: linux-user/host/riscv32/
 F: linux-user/host/riscv64/
 
+RISC-V XVentanaCondOps extension
+M: Philipp Tomsich 
+L: qemu-ri...@nongnu.org
+S: Supported
+F: target/riscv/XVentanaCondOps.decode
+F: target/riscv/insn_trans/trans_xventanacondops.c.inc
+
 RENESAS RX CPUs
 R: Yoshinori Sato 
 S: Orphan
-- 
2.33.1




[PATCH v4 3/7] target/riscv: access configuration through cfg_ptr in DisasContext

2022-01-30 Thread Philipp Tomsich
The implementation in trans_{rvi,rvv,rvzfh}.c.inc accesses the shallow
copies (in DisasContext) of some of the elements available in the
RISCVCPUConfig structure.  This commit redirects accesses to use the
cfg_ptr copied into DisasContext and removes the shallow copies.

Signed-off-by: Philipp Tomsich 
Suggested-by: Richard Henderson 

---

(no changes since v3)

Changes in v3:
- (new patch) test extension-availability through cfg_ptr in
  DisasContext, removing the fields that have been copied into
  DisasContext directly

 target/riscv/insn_trans/trans_rvi.c.inc   |   2 +-
 target/riscv/insn_trans/trans_rvv.c.inc   | 104 +++---
 target/riscv/insn_trans/trans_rvzfh.c.inc |   4 +-
 target/riscv/translate.c  |  14 ---
 4 files changed, 55 insertions(+), 69 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
b/target/riscv/insn_trans/trans_rvi.c.inc
index 3cd1b3f877..f1342f30f8 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -806,7 +806,7 @@ static bool trans_fence(DisasContext *ctx, arg_fence *a)
 
 static bool trans_fence_i(DisasContext *ctx, arg_fence_i *a)
 {
-if (!ctx->ext_ifencei) {
+if (!ctx->cfg_ptr->ext_ifencei) {
 return false;
 }
 
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index f85a9e83b4..ff09e345ad 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -74,7 +74,7 @@ static bool require_zve32f(DisasContext *s)
 }
 
 /* Zve32f doesn't support FP64. (Section 18.2) */
-return s->ext_zve32f ? s->sew <= MO_32 : true;
+return s->cfg_ptr->ext_zve32f ? s->sew <= MO_32 : true;
 }
 
 static bool require_scale_zve32f(DisasContext *s)
@@ -85,7 +85,7 @@ static bool require_scale_zve32f(DisasContext *s)
 }
 
 /* Zve32f doesn't support FP64. (Section 18.2) */
-return s->ext_zve64f ? s->sew <= MO_16 : true;
+return s->cfg_ptr->ext_zve64f ? s->sew <= MO_16 : true;
 }
 
 static bool require_zve64f(DisasContext *s)
@@ -96,7 +96,7 @@ static bool require_zve64f(DisasContext *s)
 }
 
 /* Zve64f doesn't support FP64. (Section 18.2) */
-return s->ext_zve64f ? s->sew <= MO_32 : true;
+return s->cfg_ptr->ext_zve64f ? s->sew <= MO_32 : true;
 }
 
 static bool require_scale_zve64f(DisasContext *s)
@@ -107,7 +107,7 @@ static bool require_scale_zve64f(DisasContext *s)
 }
 
 /* Zve64f doesn't support FP64. (Section 18.2) */
-return s->ext_zve64f ? s->sew <= MO_16 : true;
+return s->cfg_ptr->ext_zve64f ? s->sew <= MO_16 : true;
 }
 
 /* Destination vector register group cannot overlap source mask register. */
@@ -174,7 +174,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, 
TCGv s2)
 TCGv s1, dst;
 
 if (!require_rvv(s) ||
-!(has_ext(s, RVV) || s->ext_zve32f || s->ext_zve64f)) {
+!(has_ext(s, RVV) || s->cfg_ptr->ext_zve32f || 
s->cfg_ptr->ext_zve64f)) {
 return false;
 }
 
@@ -210,7 +210,7 @@ static bool do_vsetivli(DisasContext *s, int rd, TCGv s1, 
TCGv s2)
 TCGv dst;
 
 if (!require_rvv(s) ||
-!(has_ext(s, RVV) || s->ext_zve32f || s->ext_zve64f)) {
+!(has_ext(s, RVV) || s->cfg_ptr->ext_zve32f || 
s->cfg_ptr->ext_zve64f)) {
 return false;
 }
 
@@ -248,7 +248,7 @@ static bool trans_vsetivli(DisasContext *s, arg_vsetivli *a)
 /* vector register offset from env */
 static uint32_t vreg_ofs(DisasContext *s, int reg)
 {
-return offsetof(CPURISCVState, vreg) + reg * s->vlen / 8;
+return offsetof(CPURISCVState, vreg) + reg * s->cfg_ptr->vlen / 8;
 }
 
 /* check functions */
@@ -318,7 +318,7 @@ static bool vext_check_st_index(DisasContext *s, int vd, 
int vs2, int nf,
  * when XLEN=32. (Section 18.2)
  */
 if (get_xl(s) == MXL_RV32) {
-ret &= (!has_ext(s, RVV) && s->ext_zve64f ? eew != MO_64 : true);
+ret &= (!has_ext(s, RVV) && s->cfg_ptr->ext_zve64f ? eew != MO_64 : 
true);
 }
 
 return ret;
@@ -454,7 +454,7 @@ static bool vext_wide_check_common(DisasContext *s, int vd, 
int vm)
 {
 return (s->lmul <= 2) &&
(s->sew < MO_64) &&
-   ((s->sew + 1) <= (s->elen >> 4)) &&
+   ((s->sew + 1) <= (s->cfg_ptr->elen >> 4)) &&
require_align(vd, s->lmul + 1) &&
require_vm(vm, vd);
 }
@@ -482,7 +482,7 @@ static bool vext_narrow_check_common(DisasContext *s, int 
vd, int vs2,
 {
 return (s->lmul <= 2) &&
(s->sew < MO_64) &&
-   ((s->sew + 1) <= (s->elen >> 4)) &&
+   ((s->sew + 1) <= (s->cfg_ptr->elen >> 4)) &&
require_align(vs2, s->lmul + 1) &&
require_align(vd, s->lmul) &&
require_vm(vm, vd);
@@ -661,7 +661,7 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
  * The first part is vlen in bytes, encoded in maxsz of simd_desc.
  * The second part is lmul, encoded in data of simd_desc.
   

[PATCH v4 1/7] target/riscv: refactor (anonymous struct) RISCVCPU.cfg into 'struct RISCVCPUConfig'

2022-01-30 Thread Philipp Tomsich
Signed-off-by: Philipp Tomsich 
Suggested-by: Richard Henderson 

---

Changes in v4:
- use a typedef into 'RISCVCPUConfig' (instead of the explicit
  'struct RISCVCPUConfig') to comply with the coding standard
  (as suggested in Richard's review of v3)

Changes in v3:
- (new patch) refactor 'struct RISCVCPUConfig'

 target/riscv/cpu.h | 78 --
 1 file changed, 41 insertions(+), 37 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 55635d68d5..1175915c0d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -303,6 +303,46 @@ struct RISCVCPUClass {
 DeviceReset parent_reset;
 };
 
+struct RISCVCPUConfig {
+bool ext_i;
+bool ext_e;
+bool ext_g;
+bool ext_m;
+bool ext_a;
+bool ext_f;
+bool ext_d;
+bool ext_c;
+bool ext_s;
+bool ext_u;
+bool ext_h;
+bool ext_j;
+bool ext_v;
+bool ext_zba;
+bool ext_zbb;
+bool ext_zbc;
+bool ext_zbs;
+bool ext_counters;
+bool ext_ifencei;
+bool ext_icsr;
+bool ext_zfh;
+bool ext_zfhmin;
+bool ext_zve32f;
+bool ext_zve64f;
+
+char *priv_spec;
+char *user_spec;
+char *bext_spec;
+char *vext_spec;
+uint16_t vlen;
+uint16_t elen;
+bool mmu;
+bool pmp;
+bool epmp;
+uint64_t resetvec;
+};
+
+typedef struct RISCVCPUConfig RISCVCPUConfig;
+
 /**
  * RISCVCPU:
  * @env: #CPURISCVState
@@ -320,43 +360,7 @@ struct RISCVCPU {
 char *dyn_vreg_xml;
 
 /* Configuration Settings */
-struct {
-bool ext_i;
-bool ext_e;
-bool ext_g;
-bool ext_m;
-bool ext_a;
-bool ext_f;
-bool ext_d;
-bool ext_c;
-bool ext_s;
-bool ext_u;
-bool ext_h;
-bool ext_j;
-bool ext_v;
-bool ext_zba;
-bool ext_zbb;
-bool ext_zbc;
-bool ext_zbs;
-bool ext_counters;
-bool ext_ifencei;
-bool ext_icsr;
-bool ext_zfh;
-bool ext_zfhmin;
-bool ext_zve32f;
-bool ext_zve64f;
-
-char *priv_spec;
-char *user_spec;
-char *bext_spec;
-char *vext_spec;
-uint16_t vlen;
-uint16_t elen;
-bool mmu;
-bool pmp;
-bool epmp;
-uint64_t resetvec;
-} cfg;
+RISCVCPUConfig cfg;
 };
 
 static inline int riscv_has_ext(CPURISCVState *env, target_ulong ext)
-- 
2.33.1




[PATCH v4 4/7] target/riscv: access cfg structure through DisasContext

2022-01-30 Thread Philipp Tomsich
The Zb[abcs] support code still uses the RISCV_CPU macros to access
the configuration information (i.e., check whether an extension is
available/enabled).  Now that we provide this information directly
from DisasContext, we can access this directly via the cfg_ptr field.

Signed-off-by: Philipp Tomsich 
Suggested-by: Richard Henderson 

---

(no changes since v3)

Changes in v3:
- (new patch) change Zb[abcs] implementation to use cfg_ptr (copied
  into DisasContext) instead of going throuhg RISCV_CPU

 target/riscv/insn_trans/trans_rvb.c.inc | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvb.c.inc 
b/target/riscv/insn_trans/trans_rvb.c.inc
index 810431a1d6..f9bd3b7ec4 100644
--- a/target/riscv/insn_trans/trans_rvb.c.inc
+++ b/target/riscv/insn_trans/trans_rvb.c.inc
@@ -19,25 +19,25 @@
  */
 
 #define REQUIRE_ZBA(ctx) do {\
-if (!RISCV_CPU(ctx->cs)->cfg.ext_zba) {  \
+if (ctx->cfg_ptr->ext_zba) { \
 return false;\
 }\
 } while (0)
 
 #define REQUIRE_ZBB(ctx) do {\
-if (!RISCV_CPU(ctx->cs)->cfg.ext_zbb) {  \
+if (ctx->cfg_ptr->ext_zbb) { \
 return false;\
 }\
 } while (0)
 
 #define REQUIRE_ZBC(ctx) do {\
-if (!RISCV_CPU(ctx->cs)->cfg.ext_zbc) {  \
+if (ctx->cfg_ptr->ext_zbc) { \
 return false;\
 }\
 } while (0)
 
 #define REQUIRE_ZBS(ctx) do {\
-if (!RISCV_CPU(ctx->cs)->cfg.ext_zbs) {  \
+if (ctx->cfg_ptr->ext_zbs) { \
 return false;\
 }\
 } while (0)
-- 
2.33.1




[PATCH v4 6/7] target/riscv: Add XVentanaCondOps custom extension

2022-01-30 Thread Philipp Tomsich
This adds the decoder and translation for the XVentanaCondOps custom
extension (vendor-defined by Ventana Micro Systems), which is
documented at 
https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf

This commit then also adds a guard-function (has_XVentanaCondOps_p)
and the decoder function to the table of decoders, enabling the
support for the XVentanaCondOps extension.

Signed-off-by: Philipp Tomsich 
Reviewed-by: Richard Henderson 

---

(no changes since v3)

Changes in v3:
- rename to trans_xventanacondops.c.inc (i.e. with the '.c')
- (in MATERIALISE_EXT_PREDICATE) don't annotate the predicate function
  for testing the availability of individual extensions as 'inline'
  and don't make CPURISCVState* visible to these predicate functions

Changes in v2:
- Split off decode table into XVentanaCondOps.decode
- Wire up XVentanaCondOps in the decoder-table

 target/riscv/XVentanaCondOps.decode   | 25 
 target/riscv/cpu.c|  3 ++
 target/riscv/cpu.h|  3 ++
 .../insn_trans/trans_xventanacondops.c.inc| 39 +++
 target/riscv/meson.build  |  1 +
 target/riscv/translate.c  | 12 ++
 6 files changed, 83 insertions(+)
 create mode 100644 target/riscv/XVentanaCondOps.decode
 create mode 100644 target/riscv/insn_trans/trans_xventanacondops.c.inc

diff --git a/target/riscv/XVentanaCondOps.decode 
b/target/riscv/XVentanaCondOps.decode
new file mode 100644
index 00..5aef7c3d72
--- /dev/null
+++ b/target/riscv/XVentanaCondOps.decode
@@ -0,0 +1,25 @@
+#
+# RISC-V translation routines for the XVentanaCondOps extension
+#
+# Copyright (c) 2022 Dr. Philipp Tomsich, philipp.toms...@vrull.eu
+#
+# SPDX-License-Identifier: LGPL-2.1-or-later
+#
+# Reference: VTx-family custom instructions
+#Custom ISA extensions for Ventana Micro Systems RISC-V cores
+#
(https://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf)
+
+# Fields
+%rs2  20:5
+%rs1  15:5
+%rd7:5
+
+# Argument sets
+rd rs1 rs2  !extern
+
+# Formats
+@r ...  . . ... . ... %rs2 %rs1 
%rd
+
+# *** RV64 Custom-3 Extension ***
+vt_maskc   000  . . 110 . 011 @r
+vt_maskcn  000  . . 111 . 011 @r
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1cb0436187..6df07b8289 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -734,6 +734,9 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_BOOL("zbc", RISCVCPU, cfg.ext_zbc, true),
 DEFINE_PROP_BOOL("zbs", RISCVCPU, cfg.ext_zbs, true),
 
+/* Vendor-specific custom extensions */
+DEFINE_PROP_BOOL("xventanacondops", RISCVCPU, cfg.ext_XVentanaCondOps, 
false),
+
 /* These are experimental so mark with 'x-' */
 DEFINE_PROP_BOOL("x-j", RISCVCPU, cfg.ext_j, false),
 /* ePMP 0.9.3 */
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 1175915c0d..aacc997d56 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -329,6 +329,9 @@ struct RISCVCPUConfig {
 bool ext_zve32f;
 bool ext_zve64f;
 
+/* Vendor-specific custom extensions */
+bool ext_XVentanaCondOps;
+
 char *priv_spec;
 char *user_spec;
 char *bext_spec;
diff --git a/target/riscv/insn_trans/trans_xventanacondops.c.inc 
b/target/riscv/insn_trans/trans_xventanacondops.c.inc
new file mode 100644
index 00..b8a5d031b5
--- /dev/null
+++ b/target/riscv/insn_trans/trans_xventanacondops.c.inc
@@ -0,0 +1,39 @@
+/*
+ * RISC-V translation routines for the XVentanaCondOps extension.
+ *
+ * Copyright (c) 2021-2022 VRULL GmbH.
+ *
+ * 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 .
+ */
+
+static bool gen_condmask(DisasContext *ctx, arg_r *a, TCGCond cond)
+{
+TCGv dest = dest_gpr(ctx, a->rd);
+TCGv src1 = get_gpr(ctx, a->rs1, EXT_NONE);
+TCGv src2 = get_gpr(ctx, a->rs2, EXT_NONE);
+
+tcg_gen_movcond_tl(cond, dest, src2, ctx->zero, src1, ctx->zero);
+
+gen_set_gpr(ctx, a->rd, dest);
+return true;
+}
+
+static bool trans_vt_maskc(DisasContext *ctx, arg_r *a)
+{
+return gen_condmask(ctx, a, TCG_COND_NE);
+}
+
+static bool trans_vt_maskcn(DisasContext *ctx, arg_r *a)
+{
+return gen_condmask(ctx, a, TCG_COND_EQ);
+}
diff --git 

[PATCH v4 2/7] target/riscv: riscv_tr_init_disas_context: copy pointer-to-cfg into cfg_ptr

2022-01-30 Thread Philipp Tomsich
As the number of extensions is growing, copying them individiually
into the DisasContext will scale less and less... instead we populate
a pointer to the RISCVCPUConfig structure in the DisasContext.

This adds an extra indirection when checking for the availability of
an extension (compared to copying the fields into DisasContext).
While not a performance problem today, we can always (shallow) copy
the entire structure into the DisasContext (instead of putting a
pointer to it) if this is ever deemed necessary.

Signed-off-by: Philipp Tomsich 
Suggested-by: Richard Henderson 

---

(no changes since v3)

Changes in v3:
- (new patch) copy pointer to element cfg into DisasContext

 target/riscv/translate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index f0bbe80875..22d09af2df 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -76,6 +76,7 @@ typedef struct DisasContext {
 int frm;
 RISCVMXL ol;
 bool virt_enabled;
+const struct RISCVCPUConfig *cfg_ptr;
 bool ext_ifencei;
 bool ext_zfh;
 bool ext_zfhmin;
@@ -908,6 +909,7 @@ static void riscv_tr_init_disas_context(DisasContextBase 
*dcbase, CPUState *cs)
 #endif
 ctx->misa_ext = env->misa_ext;
 ctx->frm = -1;  /* unknown rounding mode */
+ctx->cfg_ptr = &(cpu->cfg);
 ctx->ext_ifencei = cpu->cfg.ext_ifencei;
 ctx->ext_zfh = cpu->cfg.ext_zfh;
 ctx->ext_zfhmin = cpu->cfg.ext_zfhmin;
-- 
2.33.1




[PATCH v4 5/7] target/riscv: iterate over a table of decoders

2022-01-30 Thread Philipp Tomsich
To split up the decoder into multiple functions (both to support
vendor-specific opcodes in separate files and to simplify maintenance
of orthogonal extensions), this changes decode_op to iterate over a
table of decoders predicated on guard functions.

This commit only adds the new structure and the table, allowing for
the easy addition of additional decoders in the future.

Signed-off-by: Philipp Tomsich 
---

Changes in v4:
- add braces to comply with coding standard (as suggested by Richard)
- merge the two if-statements to reduce clutter after (now that the
  braces have been added)

Changes in v3:
- expose only the DisasContext* to predicate functions
- mark the table of decoder functions as static
- drop the inline from always_true_p, until the need arises (i.e.,
  someone finds a use for it and calls it directly)
- rewrite to drop the 'handled' temporary in iterating over the
  decoder table, removing the assignment in the condition of the if

Changes in v2:
- (new patch) iterate over a table of guarded decoder functions

 target/riscv/translate.c | 32 +++-
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 441690846c..2742c32f1c 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -111,6 +111,11 @@ static inline bool has_ext(DisasContext *ctx, uint32_t ext)
 return ctx->misa_ext & ext;
 }
 
+static bool always_true_p(DisasContext *ctx  __attribute__((__unused__)))
+{
+return true;
+}
+
 #ifdef TARGET_RISCV32
 #define get_xl(ctx)MXL_RV32
 #elif defined(CONFIG_USER_ONLY)
@@ -855,15 +860,26 @@ static uint32_t opcode_at(DisasContextBase *dcbase, 
target_ulong pc)
 
 static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
 {
-/* check for compressed insn */
+/*
+ * A table with predicate (i.e., guard) functions and decoder functions
+ * that are tested in-order until a decoder matches onto the opcode.
+ */
+static const struct {
+bool (*guard_func)(DisasContext *);
+bool (*decode_func)(DisasContext *, uint32_t);
+} decoders[] = {
+{ always_true_p,  decode_insn32 },
+};
+
+/* Check for compressed insn */
 if (extract16(opcode, 0, 2) != 3) {
 if (!has_ext(ctx, RVC)) {
 gen_exception_illegal(ctx);
 } else {
 ctx->opcode = opcode;
 ctx->pc_succ_insn = ctx->base.pc_next + 2;
-if (!decode_insn16(ctx, opcode)) {
-gen_exception_illegal(ctx);
+if (decode_insn16(ctx, opcode)) {
+return;
 }
 }
 } else {
@@ -873,10 +889,16 @@ static void decode_opc(CPURISCVState *env, DisasContext 
*ctx, uint16_t opcode)
  ctx->base.pc_next + 2));
 ctx->opcode = opcode32;
 ctx->pc_succ_insn = ctx->base.pc_next + 4;
-if (!decode_insn32(ctx, opcode32)) {
-gen_exception_illegal(ctx);
+
+for (size_t i = 0; i < ARRAY_SIZE(decoders); ++i) {
+if (decoders[i].guard_func(ctx) &&
+decoders[i].decode_func(ctx, opcode32)) {
+return;
+}
 }
 }
+
+gen_exception_illegal(ctx);
 }
 
 static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
-- 
2.33.1




[PATCH v4 0/7] target/riscv: Add XVentanaCondOps and supporting infrastructure changes

2022-01-30 Thread Philipp Tomsich


In adding our first X-extension (i.e., vendor-defined) on RISC-V with
XVentanaCondOps, we need to add a few instructure improvements to make
it easier to add similar vendor-defined extensions in the future:
- refactor access to the cfg->ext_* fields by making a pointer to the
  cfg structure (as cfg_ptr) available via DisasContext
- add a table-based list of decoders to invoke, each being guarded by
  a guard/predicate-function, that can be used to either add vendor
  extensions, large extensions or override (by listing the decoder
  before the one for standard extensions) patterns to handle errata


Changes in v4:
- use a typedef into 'RISCVCPUConfig' (instead of the explicit
  'struct RISCVCPUConfig') to comply with the coding standard
  (as suggested in Richard's review of v3)
- add braces to comply with coding standard (as suggested by Richard)
- merge the two if-statements to reduce clutter after (now that the
  braces have been added)

Changes in v3:
- (new patch) refactor 'struct RISCVCPUConfig'
- (new patch) copy pointer to element cfg into DisasContext
- (new patch) test extension-availability through cfg_ptr in
  DisasContext, removing the fields that have been copied into
  DisasContext directly
- (new patch) change Zb[abcs] implementation to use cfg_ptr (copied
  into DisasContext) instead of going throuhg RISCV_CPU
- expose only the DisasContext* to predicate functions
- mark the table of decoder functions as static
- drop the inline from always_true_p, until the need arises (i.e.,
  someone finds a use for it and calls it directly)
- rewrite to drop the 'handled' temporary in iterating over the
  decoder table, removing the assignment in the condition of the if
- rename to trans_xventanacondops.c.inc (i.e. with the '.c')
- (in MATERIALISE_EXT_PREDICATE) don't annotate the predicate function
  for testing the availability of individual extensions as 'inline'
  and don't make CPURISCVState* visible to these predicate functions
- add a MAINTAINERS entry for XVentanaCondOps

Changes in v2:
- (new patch) iterate over a table of guarded decoder functions
- Split off decode table into XVentanaCondOps.decode
- Wire up XVentanaCondOps in the decoder-table

Philipp Tomsich (7):
  target/riscv: refactor (anonymous struct) RISCVCPU.cfg into 'struct
RISCVCPUConfig'
  target/riscv: riscv_tr_init_disas_context: copy pointer-to-cfg into
cfg_ptr
  target/riscv: access configuration through cfg_ptr in DisasContext
  target/riscv: access cfg structure through DisasContext
  target/riscv: iterate over a table of decoders
  target/riscv: Add XVentanaCondOps custom extension
  target/riscv: add a MAINTAINERS entry for XVentanaCondOps

 MAINTAINERS   |   7 ++
 target/riscv/XVentanaCondOps.decode   |  25 +
 target/riscv/cpu.c|   3 +
 target/riscv/cpu.h|  81 +++---
 target/riscv/insn_trans/trans_rvb.c.inc   |   8 +-
 target/riscv/insn_trans/trans_rvi.c.inc   |   2 +-
 target/riscv/insn_trans/trans_rvv.c.inc   | 104 +-
 target/riscv/insn_trans/trans_rvzfh.c.inc |   4 +-
 .../insn_trans/trans_xventanacondops.c.inc|  39 +++
 target/riscv/meson.build  |   1 +
 target/riscv/translate.c  |  60 ++
 11 files changed, 219 insertions(+), 115 deletions(-)
 create mode 100644 target/riscv/XVentanaCondOps.decode
 create mode 100644 target/riscv/insn_trans/trans_xventanacondops.c.inc

-- 
2.33.1




Re: [PATCH v1 1/6] hw/arm/xlnx-zynqmp: Add unimplemented SERDES area

2022-01-30 Thread Philippe Mathieu-Daudé via

On 31/1/22 00:12, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add unimplemented SERDES area.

Signed-off-by: Edgar E. Iglesias 
---
  include/hw/arm/xlnx-zynqmp.h | 2 +-
  hw/arm/xlnx-zynqmp.c | 4 
  2 files changed, 5 insertions(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 



Re: [PATCH v1 6/6] hw/arm/xlnx-zynqmp: Connect the ZynqMP APU Control

2022-01-30 Thread Philippe Mathieu-Daudé via

On 31/1/22 00:12, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Connect the ZynqMP APU Control device.

Signed-off-by: Edgar E. Iglesias 
---
  include/hw/arm/xlnx-zynqmp.h |  4 +++-
  hw/arm/xlnx-zynqmp.c | 25 +++--
  2 files changed, 26 insertions(+), 3 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 



Re: [PATCH v1 4/6] hw/arm/xlnx-zynqmp: Connect the ZynqMP CRF

2022-01-30 Thread Philippe Mathieu-Daudé via

On 31/1/22 00:12, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Connect the ZynqMP CRF - Clock Reset FPD device.

Signed-off-by: Edgar E. Iglesias 
---
  include/hw/arm/xlnx-zynqmp.h |  2 ++
  hw/arm/xlnx-zynqmp.c | 16 
  2 files changed, 18 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé 



Re: [PATCH v1 5/6] hw/misc: Add a model of the Xilinx ZynqMP APU Control

2022-01-30 Thread Philippe Mathieu-Daudé via

On 31/1/22 00:12, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add a model of the Xilinx ZynqMP APU Control.

Signed-off-by: Edgar E. Iglesias 
---
  include/hw/misc/xlnx-zynqmp-apu-ctrl.h |  91 +
  hw/misc/xlnx-zynqmp-apu-ctrl.c | 257 +
  hw/misc/meson.build|   1 +
  3 files changed, 349 insertions(+)
  create mode 100644 include/hw/misc/xlnx-zynqmp-apu-ctrl.h
  create mode 100644 hw/misc/xlnx-zynqmp-apu-ctrl.c

diff --git a/include/hw/misc/xlnx-zynqmp-apu-ctrl.h 
b/include/hw/misc/xlnx-zynqmp-apu-ctrl.h
new file mode 100644
index 00..44bf264cef
--- /dev/null
+++ b/include/hw/misc/xlnx-zynqmp-apu-ctrl.h
@@ -0,0 +1,91 @@
+/*
+ * QEMU model of ZynqMP APU Control.
+ *
+ * Copyright (c) 2013-2022 Xilinx Inc
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Written by Peter Crosthwaite  and
+ * Edgar E. Iglesias 
+ *
+ */
+
+#include "hw/sysbus.h"
+#include "hw/register.h"
+#include "target/arm/cpu.h"
+
+#define TYPE_XLNX_ZYNQMP_APU_CTRL "xlnx.apu-ctrl"
+
+#define XLNX_ZYNQMP_APU(obj) \
+ OBJECT_CHECK(XlnxZynqMPAPUCtrl, (obj), TYPE_XLNX_ZYNQMP_APU_CTRL)

...


+#define APU_R_MAX ((R_PWRSTAT) + 1)
+
+#define NUM_CPUS 4


Hmm isn't it APU_MAX_CPU?


+typedef struct XlnxZynqMPAPUCtrl {
+SysBusDevice busdev;
+
+ARMCPU *cpus[NUM_CPUS];
+/* WFIs towards PMU. */
+qemu_irq wfi_out[4];
+/* CPU Power status towards INTC Redirect. */
+qemu_irq cpu_power_status[4];
+qemu_irq irq_imr;
+
+uint8_t cpu_pwrdwn_req;
+uint8_t cpu_in_wfi;
+
+RegisterInfoArray *reg_array;
+uint32_t regs[APU_R_MAX];
+RegisterInfo regs_info[APU_R_MAX];
+} XlnxZynqMPAPUCtrl;




Re: [PATCH v1 3/6] hw/misc: Add a model of the Xilinx ZynqMP CRF

2022-01-30 Thread Philippe Mathieu-Daudé via

On 31/1/22 00:12, Edgar E. Iglesias wrote:

From: "Edgar E. Iglesias" 

Add a model of the Xilinx ZynqMP CRF. At the moment this
is mostly a stub model.

Signed-off-by: Edgar E. Iglesias 
---
  include/hw/misc/xlnx-zynqmp-crf.h | 209 +++
  hw/misc/xlnx-zynqmp-crf.c | 270 ++
  hw/misc/meson.build   |   1 +
  3 files changed, 480 insertions(+)
  create mode 100644 include/hw/misc/xlnx-zynqmp-crf.h
  create mode 100644 hw/misc/xlnx-zynqmp-crf.c


Reviewed-by: Philippe Mathieu-Daudé 



Re: [PATCH v3 2/3] memory: Make memory_region_is_mapped() succeed when mapped via an alias

2022-01-30 Thread Philippe Mathieu-Daudé via

Hi Niek!

(+Mark FYI)

On 30/1/22 23:50, Niek Linnenbank wrote:

Hi David,

While I realize my response is quite late, I wanted to report this error 
I found when running the acceptance

tests for the orangepi-pc machine using avocado:


Unfortunately I only run the full SD/MMC tests when I send a SD/MMC pull
request, so missed that here.

Basically the two tests freeze during the part where the U-Boot 
bootloader needs to detect the amount of memory. We model this in the 
hw/misc/allwinner-h3-dramc.c file.
And when running the machine manually it shows an assert on 
'alias->mapped_via_alias >= 0'. When running manually via gdb, I was 
able to collect this backtrace:


$ gdb ./build/qemu-system-arm
...
gdb) run -M orangepi-pc -nographic 
./Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img

...
U-Boot SPL 2020.04-armbian (Sep 02 2020 - 10:16:13 +0200)
DRAM:
qemu-system-arm: ../softmmu/memory.c:2588: memory_region_del_subregion: 
Assertion `alias->mapped_via_alias >= 0' failed.

...

So it seems that the hw/misc/allwinner-h3-dramc.c file is using the call 
memory_region_set_address, where internally we are calling 
memory_region_del_subregion.
The allwinner-h3-dramc.c file does use 
memory_region_add_subregion_overlap once in the realize function, but 
might use the memory_region_set_address multiple times.
It looks to me this is the path where the assert comes in. If I revert 
this patch on current master, the machine boots without the assertion.


Would you be able to help out how we can best resolve this? Ofcourse, if 
there is anything needed to be changed on the allwinner-h3-dramc.c file, 
I would be happy to prepare a patch for that.


David's patch LGTM and I think your model might be somehow abusing the
memory API, but I'd like to read on the DRAMCOM Control Register to
understand the allwinner_h3_dramc_map_rows() logic. I couldn't find a
reference looking at Allwinner_H3_Datasheet_V1.2.pdf.
I wonder if we could ignore implementing it.

Your use case is typically what I tried to solve with this model:
https://lore.kernel.org/qemu-devel/20210419094329.1402767-2-f4...@amsat.org/

In your case, @span_size is your amount of DRAM, and @region_size is the
area u-boot is scanning (and @offset is zero).
Could that work, or is DRAMCOM doing much more?

Thanks,

Phil.

P.D. reference to documentation welcome :)



[PATCH v1 3/6] hw/misc: Add a model of the Xilinx ZynqMP CRF

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

Add a model of the Xilinx ZynqMP CRF. At the moment this
is mostly a stub model.

Signed-off-by: Edgar E. Iglesias 
---
 include/hw/misc/xlnx-zynqmp-crf.h | 209 +++
 hw/misc/xlnx-zynqmp-crf.c | 270 ++
 hw/misc/meson.build   |   1 +
 3 files changed, 480 insertions(+)
 create mode 100644 include/hw/misc/xlnx-zynqmp-crf.h
 create mode 100644 hw/misc/xlnx-zynqmp-crf.c

diff --git a/include/hw/misc/xlnx-zynqmp-crf.h 
b/include/hw/misc/xlnx-zynqmp-crf.h
new file mode 100644
index 00..b173ea4a08
--- /dev/null
+++ b/include/hw/misc/xlnx-zynqmp-crf.h
@@ -0,0 +1,209 @@
+/*
+ * QEMU model of the CRF - Clock Reset FPD.
+ *
+ * Copyright (c) 2022 Xilinx Inc.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ * Written by Edgar E. Iglesias 
+ */
+
+#include "hw/sysbus.h"
+#include "hw/register.h"
+
+#define TYPE_XLNX_ZYNQMP_CRF "xlnx.zynqmp_crf"
+
+#define XILINX_CRF(obj) \
+ OBJECT_CHECK(XlnxZynqMPCRF, (obj), TYPE_XLNX_ZYNQMP_CRF)
+
+REG32(ERR_CTRL, 0x0)
+FIELD(ERR_CTRL, SLVERR_ENABLE, 0, 1)
+REG32(IR_STATUS, 0x4)
+FIELD(IR_STATUS, ADDR_DECODE_ERR, 0, 1)
+REG32(IR_MASK, 0x8)
+FIELD(IR_MASK, ADDR_DECODE_ERR, 0, 1)
+REG32(IR_ENABLE, 0xc)
+FIELD(IR_ENABLE, ADDR_DECODE_ERR, 0, 1)
+REG32(IR_DISABLE, 0x10)
+FIELD(IR_DISABLE, ADDR_DECODE_ERR, 0, 1)
+REG32(CRF_WPROT, 0x1c)
+FIELD(CRF_WPROT, ACTIVE, 0, 1)
+REG32(APLL_CTRL, 0x20)
+FIELD(APLL_CTRL, POST_SRC, 24, 3)
+FIELD(APLL_CTRL, PRE_SRC, 20, 3)
+FIELD(APLL_CTRL, CLKOUTDIV, 17, 1)
+FIELD(APLL_CTRL, DIV2, 16, 1)
+FIELD(APLL_CTRL, FBDIV, 8, 7)
+FIELD(APLL_CTRL, BYPASS, 3, 1)
+FIELD(APLL_CTRL, RESET, 0, 1)
+REG32(APLL_CFG, 0x24)
+FIELD(APLL_CFG, LOCK_DLY, 25, 7)
+FIELD(APLL_CFG, LOCK_CNT, 13, 10)
+FIELD(APLL_CFG, LFHF, 10, 2)
+FIELD(APLL_CFG, CP, 5, 4)
+FIELD(APLL_CFG, RES, 0, 4)
+REG32(APLL_FRAC_CFG, 0x28)
+FIELD(APLL_FRAC_CFG, ENABLED, 31, 1)
+FIELD(APLL_FRAC_CFG, SEED, 22, 3)
+FIELD(APLL_FRAC_CFG, ALGRTHM, 19, 1)
+FIELD(APLL_FRAC_CFG, ORDER, 18, 1)
+FIELD(APLL_FRAC_CFG, DATA, 0, 16)
+REG32(DPLL_CTRL, 0x2c)
+FIELD(DPLL_CTRL, POST_SRC, 24, 3)
+FIELD(DPLL_CTRL, PRE_SRC, 20, 3)
+FIELD(DPLL_CTRL, CLKOUTDIV, 17, 1)
+FIELD(DPLL_CTRL, DIV2, 16, 1)
+FIELD(DPLL_CTRL, FBDIV, 8, 7)
+FIELD(DPLL_CTRL, BYPASS, 3, 1)
+FIELD(DPLL_CTRL, RESET, 0, 1)
+REG32(DPLL_CFG, 0x30)
+FIELD(DPLL_CFG, LOCK_DLY, 25, 7)
+FIELD(DPLL_CFG, LOCK_CNT, 13, 10)
+FIELD(DPLL_CFG, LFHF, 10, 2)
+FIELD(DPLL_CFG, CP, 5, 4)
+FIELD(DPLL_CFG, RES, 0, 4)
+REG32(DPLL_FRAC_CFG, 0x34)
+FIELD(DPLL_FRAC_CFG, ENABLED, 31, 1)
+FIELD(DPLL_FRAC_CFG, SEED, 22, 3)
+FIELD(DPLL_FRAC_CFG, ALGRTHM, 19, 1)
+FIELD(DPLL_FRAC_CFG, ORDER, 18, 1)
+FIELD(DPLL_FRAC_CFG, DATA, 0, 16)
+REG32(VPLL_CTRL, 0x38)
+FIELD(VPLL_CTRL, POST_SRC, 24, 3)
+FIELD(VPLL_CTRL, PRE_SRC, 20, 3)
+FIELD(VPLL_CTRL, CLKOUTDIV, 17, 1)
+FIELD(VPLL_CTRL, DIV2, 16, 1)
+FIELD(VPLL_CTRL, FBDIV, 8, 7)
+FIELD(VPLL_CTRL, BYPASS, 3, 1)
+FIELD(VPLL_CTRL, RESET, 0, 1)
+REG32(VPLL_CFG, 0x3c)
+FIELD(VPLL_CFG, LOCK_DLY, 25, 7)
+FIELD(VPLL_CFG, LOCK_CNT, 13, 10)
+FIELD(VPLL_CFG, LFHF, 10, 2)
+FIELD(VPLL_CFG, CP, 5, 4)
+FIELD(VPLL_CFG, RES, 0, 4)
+REG32(VPLL_FRAC_CFG, 0x40)
+FIELD(VPLL_FRAC_CFG, ENABLED, 31, 1)
+FIELD(VPLL_FRAC_CFG, SEED, 22, 3)
+FIELD(VPLL_FRAC_CFG, ALGRTHM, 19, 1)
+FIELD(VPLL_FRAC_CFG, ORDER, 18, 1)
+FIELD(VPLL_FRAC_CFG, DATA, 0, 16)
+REG32(PLL_STATUS, 0x44)
+FIELD(PLL_STATUS, VPLL_STABLE, 5, 1)
+FIELD(PLL_STATUS, DPLL_STABLE, 4, 1)
+FIELD(PLL_STATUS, APLL_STABLE, 3, 1)
+FIELD(PLL_STATUS, VPLL_LOCK, 2, 1)
+FIELD(PLL_STATUS, DPLL_LOCK, 1, 1)
+FIELD(PLL_STATUS, APLL_LOCK, 0, 1)
+REG32(APLL_TO_LPD_CTRL, 0x48)
+FIELD(APLL_TO_LPD_CTRL, DIVISOR0, 8, 6)
+REG32(DPLL_TO_LPD_CTRL, 0x4c)
+FIELD(DPLL_TO_LPD_CTRL, DIVISOR0, 8, 6)
+REG32(VPLL_TO_LPD_CTRL, 0x50)
+FIELD(VPLL_TO_LPD_CTRL, DIVISOR0, 8, 6)
+REG32(ACPU_CTRL, 0x60)
+FIELD(ACPU_CTRL, CLKACT_HALF, 25, 1)
+FIELD(ACPU_CTRL, CLKACT_FULL, 24, 1)
+FIELD(ACPU_CTRL, DIVISOR0, 8, 6)
+FIELD(ACPU_CTRL, SRCSEL, 0, 3)
+REG32(DBG_TRACE_CTRL, 0x64)
+FIELD(DBG_TRACE_CTRL, CLKACT, 24, 1)
+FIELD(DBG_TRACE_CTRL, DIVISOR0, 8, 6)
+FIELD(DBG_TRACE_CTRL, SRCSEL, 0, 3)
+REG32(DBG_FPD_CTRL, 0x68)
+FIELD(DBG_FPD_CTRL, CLKACT, 24, 1)
+FIELD(DBG_FPD_CTRL, DIVISOR0, 8, 6)
+FIELD(DBG_FPD_CTRL, SRCSEL, 0, 3)
+REG32(DP_VIDEO_REF_CTRL, 0x70)
+FIELD(DP_VIDEO_REF_CTRL, CLKACT, 24, 1)
+FIELD(DP_VIDEO_REF_CTRL, DIVISOR1, 16, 6)
+FIELD(DP_VIDEO_REF_CTRL, DIVISOR0, 8, 6)
+FIELD(DP_VIDEO_REF_CTRL, SRCSEL, 0, 3)
+REG32(DP_AUDIO_REF_CTRL, 0x74)
+FIELD(DP_AUDIO_REF_CTRL, CLKACT, 24, 1)
+FIELD(DP_AUDIO_REF_CTRL, DIVISOR1, 16, 6)
+FIELD(DP_AUDIO_REF_CTRL, DIVISOR0, 8, 6)
+FIELD(DP_AUDIO_REF_CTRL, SRCSEL, 0, 3)
+REG32(DP_STC_REF_CTRL, 

[PATCH v1 6/6] hw/arm/xlnx-zynqmp: Connect the ZynqMP APU Control

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

Connect the ZynqMP APU Control device.

Signed-off-by: Edgar E. Iglesias 
---
 include/hw/arm/xlnx-zynqmp.h |  4 +++-
 hw/arm/xlnx-zynqmp.c | 25 +++--
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index d5a3ad3df2..05cd2128f3 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -38,6 +38,7 @@
 #include "hw/dma/xlnx_csu_dma.h"
 #include "hw/nvram/xlnx-bbram.h"
 #include "hw/nvram/xlnx-zynqmp-efuse.h"
+#include "hw/misc/xlnx-zynqmp-apu-ctrl.h"
 #include "hw/misc/xlnx-zynqmp-crf.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx-zynqmp"
@@ -85,7 +86,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(XlnxZynqMPState, XLNX_ZYNQMP)
 /*
  * Unimplemented mmio regions needed to boot some images.
  */
-#define XLNX_ZYNQMP_NUM_UNIMP_AREAS 2
+#define XLNX_ZYNQMP_NUM_UNIMP_AREAS 1
 
 struct XlnxZynqMPState {
 /*< private >*/
@@ -123,6 +124,7 @@ struct XlnxZynqMPState {
 XlnxZDMA gdma[XLNX_ZYNQMP_NUM_GDMA_CH];
 XlnxZDMA adma[XLNX_ZYNQMP_NUM_ADMA_CH];
 XlnxCSUDMA qspi_dma;
+XlnxZynqMPAPUCtrl apu_ctrl;
 XlnxZynqMPCRF crf;
 
 char *boot_cpu;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 857d3c9636..21c411cd77 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -64,7 +64,7 @@
 #define DPDMA_IRQ   116
 
 #define APU_ADDR0xfd5c
-#define APU_SIZE0x100
+#define APU_IRQ 153
 
 #define IPI_ADDR0xFF30
 #define IPI_IRQ 64
@@ -282,6 +282,27 @@ static void xlnx_zynqmp_create_efuse(XlnxZynqMPState *s, 
qemu_irq *gic)
 sysbus_connect_irq(sbd, 0, gic[EFUSE_IRQ]);
 }
 
+static void xlnx_zynqmp_create_apu_ctrl(XlnxZynqMPState *s, qemu_irq *gic)
+{
+SysBusDevice *sbd;
+int i;
+
+object_initialize_child(OBJECT(s), "apu-ctrl", >apu_ctrl,
+TYPE_XLNX_ZYNQMP_APU_CTRL);
+sbd = SYS_BUS_DEVICE(>apu_ctrl);
+
+for (i = 0; i < XLNX_ZYNQMP_NUM_APU_CPUS; i++) {
+g_autofree gchar *name = g_strdup_printf("cpu%d", i);
+
+object_property_set_link(OBJECT(>apu_ctrl), name,
+ OBJECT(>apu_cpu[i]), _abort);
+}
+
+sysbus_realize(sbd, _fatal);
+sysbus_mmio_map(sbd, 0, APU_ADDR);
+sysbus_connect_irq(sbd, 0, gic[APU_IRQ]);
+}
+
 static void xlnx_zynqmp_create_crf(XlnxZynqMPState *s, qemu_irq *gic)
 {
 SysBusDevice *sbd;
@@ -301,7 +322,6 @@ static void xlnx_zynqmp_create_unimp_mmio(XlnxZynqMPState 
*s)
 hwaddr base;
 hwaddr size;
 } unimp_areas[ARRAY_SIZE(s->mr_unimp)] = {
-{ .name = "apu", APU_ADDR, APU_SIZE },
 { .name = "serdes", SERDES_ADDR, SERDES_SIZE },
 };
 unsigned int nr;
@@ -697,6 +717,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 
 xlnx_zynqmp_create_bbram(s, gic_spi);
 xlnx_zynqmp_create_efuse(s, gic_spi);
+xlnx_zynqmp_create_apu_ctrl(s, gic_spi);
 xlnx_zynqmp_create_crf(s, gic_spi);
 xlnx_zynqmp_create_unimp_mmio(s);
 
-- 
2.25.1




[PATCH v1 2/6] target/arm: Make rvbar settable after realize

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

Make the rvbar property settable after realize. This is done
in preparation to model the ZynqMP's runtime configurable rvbar.

Signed-off-by: Edgar E. Iglesias 
---
 target/arm/cpu.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 5a9c02a256..e30ae088fe 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1128,9 +1128,6 @@ static Property arm_cpu_reset_cbar_property =
 static Property arm_cpu_reset_hivecs_property =
 DEFINE_PROP_BOOL("reset-hivecs", ARMCPU, reset_hivecs, false);
 
-static Property arm_cpu_rvbar_property =
-DEFINE_PROP_UINT64("rvbar", ARMCPU, rvbar, 0);
-
 #ifndef CONFIG_USER_ONLY
 static Property arm_cpu_has_el2_property =
 DEFINE_PROP_BOOL("has_el2", ARMCPU, has_el2, true);
@@ -1233,7 +1230,9 @@ void arm_cpu_post_init(Object *obj)
 }
 
 if (arm_feature(>env, ARM_FEATURE_AARCH64)) {
-qdev_property_add_static(DEVICE(obj), _cpu_rvbar_property);
+object_property_add_uint64_ptr(obj, "rvbar",
+   >rvbar,
+   OBJ_PROP_FLAG_READWRITE);
 }
 
 #ifndef CONFIG_USER_ONLY
-- 
2.25.1




[PATCH v1 4/6] hw/arm/xlnx-zynqmp: Connect the ZynqMP CRF

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

Connect the ZynqMP CRF - Clock Reset FPD device.

Signed-off-by: Edgar E. Iglesias 
---
 include/hw/arm/xlnx-zynqmp.h |  2 ++
 hw/arm/xlnx-zynqmp.c | 16 
 2 files changed, 18 insertions(+)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 99ceb8a609..d5a3ad3df2 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -38,6 +38,7 @@
 #include "hw/dma/xlnx_csu_dma.h"
 #include "hw/nvram/xlnx-bbram.h"
 #include "hw/nvram/xlnx-zynqmp-efuse.h"
+#include "hw/misc/xlnx-zynqmp-crf.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx-zynqmp"
 OBJECT_DECLARE_SIMPLE_TYPE(XlnxZynqMPState, XLNX_ZYNQMP)
@@ -122,6 +123,7 @@ struct XlnxZynqMPState {
 XlnxZDMA gdma[XLNX_ZYNQMP_NUM_GDMA_CH];
 XlnxZDMA adma[XLNX_ZYNQMP_NUM_ADMA_CH];
 XlnxCSUDMA qspi_dma;
+XlnxZynqMPCRF crf;
 
 char *boot_cpu;
 ARMCPU *boot_cpu_ptr;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index ba44e95899..857d3c9636 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -51,6 +51,9 @@
 #define QSPI_IRQ15
 #define QSPI_DMA_ADDR   0xff0f0800
 
+#define CRF_ADDR0xfd1a
+#define CRF_IRQ 120
+
 #define SERDES_ADDR 0xfd40
 #define SERDES_SIZE 0x2
 
@@ -279,6 +282,18 @@ static void xlnx_zynqmp_create_efuse(XlnxZynqMPState *s, 
qemu_irq *gic)
 sysbus_connect_irq(sbd, 0, gic[EFUSE_IRQ]);
 }
 
+static void xlnx_zynqmp_create_crf(XlnxZynqMPState *s, qemu_irq *gic)
+{
+SysBusDevice *sbd;
+
+object_initialize_child(OBJECT(s), "crf", >crf, TYPE_XLNX_ZYNQMP_CRF);
+sbd = SYS_BUS_DEVICE(>crf);
+
+sysbus_realize(sbd, _fatal);
+sysbus_mmio_map(sbd, 0, CRF_ADDR);
+sysbus_connect_irq(sbd, 0, gic[CRF_IRQ]);
+}
+
 static void xlnx_zynqmp_create_unimp_mmio(XlnxZynqMPState *s)
 {
 static const struct UnimpInfo {
@@ -682,6 +697,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 
 xlnx_zynqmp_create_bbram(s, gic_spi);
 xlnx_zynqmp_create_efuse(s, gic_spi);
+xlnx_zynqmp_create_crf(s, gic_spi);
 xlnx_zynqmp_create_unimp_mmio(s);
 
 for (i = 0; i < XLNX_ZYNQMP_NUM_GDMA_CH; i++) {
-- 
2.25.1




[PATCH v1 1/6] hw/arm/xlnx-zynqmp: Add unimplemented SERDES area

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

Add unimplemented SERDES area.

Signed-off-by: Edgar E. Iglesias 
---
 include/hw/arm/xlnx-zynqmp.h | 2 +-
 hw/arm/xlnx-zynqmp.c | 4 
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index 062e637fe4..99ceb8a609 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -84,7 +84,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(XlnxZynqMPState, XLNX_ZYNQMP)
 /*
  * Unimplemented mmio regions needed to boot some images.
  */
-#define XLNX_ZYNQMP_NUM_UNIMP_AREAS 1
+#define XLNX_ZYNQMP_NUM_UNIMP_AREAS 2
 
 struct XlnxZynqMPState {
 /*< private >*/
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 17305fe7b7..ba44e95899 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -51,6 +51,9 @@
 #define QSPI_IRQ15
 #define QSPI_DMA_ADDR   0xff0f0800
 
+#define SERDES_ADDR 0xfd40
+#define SERDES_SIZE 0x2
+
 #define DP_ADDR 0xfd4a
 #define DP_IRQ  113
 
@@ -284,6 +287,7 @@ static void xlnx_zynqmp_create_unimp_mmio(XlnxZynqMPState 
*s)
 hwaddr size;
 } unimp_areas[ARRAY_SIZE(s->mr_unimp)] = {
 { .name = "apu", APU_ADDR, APU_SIZE },
+{ .name = "serdes", SERDES_ADDR, SERDES_SIZE },
 };
 unsigned int nr;
 
-- 
2.25.1




[PATCH v1 5/6] hw/misc: Add a model of the Xilinx ZynqMP APU Control

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

Add a model of the Xilinx ZynqMP APU Control.

Signed-off-by: Edgar E. Iglesias 
---
 include/hw/misc/xlnx-zynqmp-apu-ctrl.h |  91 +
 hw/misc/xlnx-zynqmp-apu-ctrl.c | 257 +
 hw/misc/meson.build|   1 +
 3 files changed, 349 insertions(+)
 create mode 100644 include/hw/misc/xlnx-zynqmp-apu-ctrl.h
 create mode 100644 hw/misc/xlnx-zynqmp-apu-ctrl.c

diff --git a/include/hw/misc/xlnx-zynqmp-apu-ctrl.h 
b/include/hw/misc/xlnx-zynqmp-apu-ctrl.h
new file mode 100644
index 00..44bf264cef
--- /dev/null
+++ b/include/hw/misc/xlnx-zynqmp-apu-ctrl.h
@@ -0,0 +1,91 @@
+/*
+ * QEMU model of ZynqMP APU Control.
+ *
+ * Copyright (c) 2013-2022 Xilinx Inc
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Written by Peter Crosthwaite  and
+ * Edgar E. Iglesias 
+ *
+ */
+
+#include "hw/sysbus.h"
+#include "hw/register.h"
+#include "target/arm/cpu.h"
+
+#define TYPE_XLNX_ZYNQMP_APU_CTRL "xlnx.apu-ctrl"
+
+#define XLNX_ZYNQMP_APU(obj) \
+ OBJECT_CHECK(XlnxZynqMPAPUCtrl, (obj), TYPE_XLNX_ZYNQMP_APU_CTRL)
+
+REG32(APU_ERR_CTRL, 0x0)
+FIELD(APU_ERR_CTRL, PSLVERR, 0, 1)
+REG32(ISR, 0x10)
+FIELD(ISR, INV_APB, 0, 1)
+REG32(IMR, 0x14)
+FIELD(IMR, INV_APB, 0, 1)
+REG32(IEN, 0x18)
+FIELD(IEN, INV_APB, 0, 1)
+REG32(IDS, 0x1c)
+FIELD(IDS, INV_APB, 0, 1)
+REG32(CONFIG_0, 0x20)
+FIELD(CONFIG_0, CFGTE, 24, 4)
+FIELD(CONFIG_0, CFGEND, 16, 4)
+FIELD(CONFIG_0, VINITHI, 8, 4)
+FIELD(CONFIG_0, AA64NAA32, 0, 4)
+REG32(CONFIG_1, 0x24)
+FIELD(CONFIG_1, L2RSTDISABLE, 29, 1)
+FIELD(CONFIG_1, L1RSTDISABLE, 28, 1)
+FIELD(CONFIG_1, CP15DISABLE, 0, 4)
+REG32(RVBARADDR0L, 0x40)
+FIELD(RVBARADDR0L, ADDR, 2, 30)
+REG32(RVBARADDR0H, 0x44)
+FIELD(RVBARADDR0H, ADDR, 0, 8)
+REG32(RVBARADDR1L, 0x48)
+FIELD(RVBARADDR1L, ADDR, 2, 30)
+REG32(RVBARADDR1H, 0x4c)
+FIELD(RVBARADDR1H, ADDR, 0, 8)
+REG32(RVBARADDR2L, 0x50)
+FIELD(RVBARADDR2L, ADDR, 2, 30)
+REG32(RVBARADDR2H, 0x54)
+FIELD(RVBARADDR2H, ADDR, 0, 8)
+REG32(RVBARADDR3L, 0x58)
+FIELD(RVBARADDR3L, ADDR, 2, 30)
+REG32(RVBARADDR3H, 0x5c)
+FIELD(RVBARADDR3H, ADDR, 0, 8)
+REG32(ACE_CTRL, 0x60)
+FIELD(ACE_CTRL, AWQOS, 16, 4)
+FIELD(ACE_CTRL, ARQOS, 0, 4)
+REG32(SNOOP_CTRL, 0x80)
+FIELD(SNOOP_CTRL, ACE_INACT, 4, 1)
+FIELD(SNOOP_CTRL, ACP_INACT, 0, 1)
+REG32(PWRCTL, 0x90)
+FIELD(PWRCTL, CLREXMONREQ, 17, 1)
+FIELD(PWRCTL, L2FLUSHREQ, 16, 1)
+FIELD(PWRCTL, CPUPWRDWNREQ, 0, 4)
+REG32(PWRSTAT, 0x94)
+FIELD(PWRSTAT, CLREXMONACK, 17, 1)
+FIELD(PWRSTAT, L2FLUSHDONE, 16, 1)
+FIELD(PWRSTAT, DBGNOPWRDWN, 0, 4)
+
+#define APU_R_MAX ((R_PWRSTAT) + 1)
+
+#define NUM_CPUS 4
+
+typedef struct XlnxZynqMPAPUCtrl {
+SysBusDevice busdev;
+
+ARMCPU *cpus[NUM_CPUS];
+/* WFIs towards PMU. */
+qemu_irq wfi_out[4];
+/* CPU Power status towards INTC Redirect. */
+qemu_irq cpu_power_status[4];
+qemu_irq irq_imr;
+
+uint8_t cpu_pwrdwn_req;
+uint8_t cpu_in_wfi;
+
+RegisterInfoArray *reg_array;
+uint32_t regs[APU_R_MAX];
+RegisterInfo regs_info[APU_R_MAX];
+} XlnxZynqMPAPUCtrl;
diff --git a/hw/misc/xlnx-zynqmp-apu-ctrl.c b/hw/misc/xlnx-zynqmp-apu-ctrl.c
new file mode 100644
index 00..c27b8b9253
--- /dev/null
+++ b/hw/misc/xlnx-zynqmp-apu-ctrl.c
@@ -0,0 +1,257 @@
+/*
+ * QEMU model of the ZynqMP APU Control.
+ *
+ * Copyright (c) 2013-2022 Xilinx Inc
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Written by Peter Crosthwaite  and
+ * Edgar E. Iglesias 
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/log.h"
+#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "hw/register.h"
+
+#include "qemu/bitops.h"
+#include "qapi/qmp/qerror.h"
+
+#include "hw/misc/xlnx-zynqmp-apu-ctrl.h"
+
+#ifndef XILINX_ZYNQMP_APU_ERR_DEBUG
+#define XILINX_ZYNQMP_APU_ERR_DEBUG 1
+#endif
+
+static void update_wfi_out(void *opaque)
+{
+XlnxZynqMPAPUCtrl *s = XLNX_ZYNQMP_APU(opaque);
+unsigned int i, wfi_pending;
+
+wfi_pending = s->cpu_pwrdwn_req & s->cpu_in_wfi;
+for (i = 0; i < NUM_CPUS; i++) {
+qemu_set_irq(s->wfi_out[i], !!(wfi_pending & (1 << i)));
+}
+}
+
+static void zynqmp_apu_rvbar_post_write(RegisterInfo *reg, uint64_t val)
+{
+XlnxZynqMPAPUCtrl *s = XLNX_ZYNQMP_APU(reg->opaque);
+int i;
+
+for (i = 0; i < NUM_CPUS; ++i) {
+uint64_t rvbar = s->regs[R_RVBARADDR0L + 2 * i] +
+ ((uint64_t)s->regs[R_RVBARADDR0H + 2 * i] << 32);
+if (s->cpus[i]) {
+object_property_set_int(OBJECT(s->cpus[i]), "rvbar", rvbar,
+_abort);
+}
+}
+}
+
+static void zynqmp_apu_pwrctl_post_write(RegisterInfo *reg, uint64_t val)
+{
+XlnxZynqMPAPUCtrl *s = XLNX_ZYNQMP_APU(reg->opaque);
+unsigned int i, new;
+
+for (i = 0; i < NUM_CPUS; i++) {
+new = val & (1 << i);
+  

[PATCH v1 0/6] hw/arm: zynqmp: Add CRF and APU control to support PSCI

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

This adds the necessary modeling to support some of our firmware
tests at EL3 implementing PSCI (TBM). These are the test-cases
that were previously relying on QEMU's builtin PSCI emulation.

I've only tested this on top of Peter's recent PSCI emulation fixes.

Cheers,
Edgar

Edgar E. Iglesias (6):
  hw/arm/xlnx-zynqmp: Add unimplemented SERDES area
  target/arm: Make rvbar settable after realize
  hw/misc: Add a model of the Xilinx ZynqMP CRF
  hw/arm/xlnx-zynqmp: Connect the ZynqMP CRF
  hw/misc: Add a model of the Xilinx ZynqMP APU Control
  hw/arm/xlnx-zynqmp: Connect the ZynqMP APU Control

 include/hw/arm/xlnx-zynqmp.h   |   4 +
 include/hw/misc/xlnx-zynqmp-apu-ctrl.h |  91 +
 include/hw/misc/xlnx-zynqmp-crf.h  | 209 +++
 hw/arm/xlnx-zynqmp.c   |  45 -
 hw/misc/xlnx-zynqmp-apu-ctrl.c | 257 +++
 hw/misc/xlnx-zynqmp-crf.c  | 270 +
 target/arm/cpu.c   |   7 +-
 hw/misc/meson.build|   2 +
 8 files changed, 879 insertions(+), 6 deletions(-)
 create mode 100644 include/hw/misc/xlnx-zynqmp-apu-ctrl.h
 create mode 100644 include/hw/misc/xlnx-zynqmp-crf.h
 create mode 100644 hw/misc/xlnx-zynqmp-apu-ctrl.c
 create mode 100644 hw/misc/xlnx-zynqmp-crf.c

-- 
2.25.1




Re: [PATCH 3/3] isa/piix4: Resolve global variables

2022-01-30 Thread Philippe Mathieu-Daudé via

On 14/1/22 14:36, Peter Maydell wrote:

On Wed, 12 Jan 2022 at 22:02, Bernhard Beschow  wrote:


Now that piix4_set_irq's opaque parameter references own PIIX4State,
piix4_dev becomes redundant and pci_irq_levels can be moved into PIIX4State.

Signed-off-by: Bernhard Beschow 
---
  hw/isa/piix4.c| 22 +-
  include/hw/southbridge/piix.h |  2 --
  2 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index a31e9714cf..964e09cf7f 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -39,14 +39,14 @@
  #include "sysemu/runstate.h"
  #include "qom/object.h"

-PCIDevice *piix4_dev;
-
  struct PIIX4State {
  PCIDevice dev;
  qemu_irq cpu_intr;
  qemu_irq *isa;
  qemu_irq i8259[ISA_NUM_IRQS];

+int pci_irq_levels[PIIX_NUM_PIRQS];
+


I wondered how we were migrating this state, and the answer
seems to be that we aren't (and weren't before, when it was
a global variable, so this is a pre-existing bug).


Indeed the migrated VM starts with PCI IRQ levels zeroed.


Does the malta platform support migration save/load?


Maybe a "best effort" support, but not versioned machines.


We should probably add this field to the vmstate struct
(which will be a migration compatibility break, which is OK
as the malta board isn't versioned.)


Yeah good catch.

Bernhard, do you mind adding it?

Regards,

Phil.



Re: [PATCH v3 2/3] memory: Make memory_region_is_mapped() succeed when mapped via an alias

2022-01-30 Thread Niek Linnenbank
Hi David,

While I realize my response is quite late, I wanted to report this error I
found when running the acceptance
tests for the orangepi-pc machine using avocado:

ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes avocado
--show=app,console run -t machine:orangepi-pc
tests/avocado/boot_linux_console.py
...
 (4/5)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08:
-console: U-Boot SPL 2020.04-armbian (Sep 02 2020 - 10:16:13 +0200)
\console: DRAM:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'4-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08',
'logdir': '/home/fox/avocado/job-results/job-2022-01-30T23.09-af49e...
(90.64 s)
 (5/5)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
/console: U-Boot SPL 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +)
console: DRAM:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'5-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9',
'logdir': '/home/fox/avocado/job-results/job-2022-01-30T23.09-af49...
(90.64 s)
RESULTS: PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 2 |
CANCEL 0
JOB TIME   : 221.25 s

Basically the two tests freeze during the part where the U-Boot bootloader
needs to detect the amount of memory. We model this in the
hw/misc/allwinner-h3-dramc.c file.
And when running the machine manually it shows an assert on
'alias->mapped_via_alias >= 0'. When running manually via gdb, I was able
to collect this backtrace:

$ gdb ./build/qemu-system-arm
...
gdb) run -M orangepi-pc -nographic
./Armbian_20.08.1_Orangepipc_bionic_current_5.8.5.img
...
U-Boot SPL 2020.04-armbian (Sep 02 2020 - 10:16:13 +0200)
DRAM:
qemu-system-arm: ../softmmu/memory.c:2588: memory_region_del_subregion:
Assertion `alias->mapped_via_alias >= 0' failed.

Thread 4 "qemu-system-arm" received signal SIGABRT, Aborted.
[Switching to Thread 0x75f72700 (LWP 32866)]
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x77535859 in __GI_abort () at abort.c:79
#2  0x77535729 in __assert_fail_base
(fmt=0x776cb588 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
assertion=0x5642fd65 "alias->mapped_via_alias >= 0",
file=0x5642f8cd "../softmmu/memory.c", line=2588, function=)
at assert.c:92
#3  0x77546f36 in __GI___assert_fail
(assertion=0x5642fd65 "alias->mapped_via_alias >= 0",
file=0x5642f8cd "../softmmu/memory.c", line=2588,
function=0x56430690 <__PRETTY_FUNCTION__.8>
"memory_region_del_subregion") at assert.c:101
#4  0x55e587e0 in memory_region_del_subregion (mr=0x56f0bc00,
subregion=0x75fa1090) at ../softmmu/memory.c:2588
#5  0x55e589f3 in memory_region_readd_subregion (mr=0x75fa1090)
at ../softmmu/memory.c:2630
#6  0x55e58a5f in memory_region_set_address (mr=0x75fa1090,
addr=1090519040) at ../softmmu/memory.c:2642
#7  0x55ac352b in allwinner_h3_dramc_map_rows (s=0x75fa0c50,
row_bits=16 '\020', bank_bits=2 '\002', page_size=512) at
../hw/misc/allwinner-h3-dramc.c:92
#8  0x55ac36c2 in allwinner_h3_dramcom_write
(opaque=0x75fa0c50, offset=0, val=4396785, size=4) at
../hw/misc/allwinner-h3-dramc.c:131
#9  0x55e52561 in memory_region_write_accessor (mr=0x75fa11a0,
addr=0, value=0x75f710e8, size=4, shift=0, mask=4294967295, attrs=...)
at ../softmmu/memory.c:492
#10 0x55e527ad in access_with_adjusted_size (addr=0,
value=0x75f710e8, size=4, access_size_min=4, access_size_max=4,
access_fn=
0x55e52467 , mr=0x75fa11a0,
attrs=...) at ../softmmu/memory.c:554
#11 0x55e55935 in memory_region_dispatch_write (mr=0x75fa11a0,
addr=0, data=4396785, op=MO_32, attrs=...) at ../softmmu/memory.c:1514
#12 0x55f891ae in io_writex (env=0x75f7ce30,
iotlbentry=0x7fffec0275f0, mmu_idx=7, val=4396785, addr=29761536,
retaddr=140734938367479, op=MO_32) at ../accel/tcg/cputlb.c:1420
#13 0x55f8ba10 in store_helper (env=0x75f7ce30, addr=29761536,
val=4396785, oi=3623, retaddr=140734938367479, op=MO_32) at
../accel/tcg/cputlb.c:2355
#14 0x55f8bdda in full_le_stl_mmu (env=0x75f7ce30,
addr=29761536, val=4396785, oi=3623, retaddr=140734938367479) at
../accel/tcg/cputlb.c:2443
#15 0x55f8be16 in helper_le_stl_mmu (env=0x75f7ce30,
addr=29761536, val=4396785, oi=3623, retaddr=140734938367479) at
../accel/tcg/cputlb.c:2449
#16 0x7fff680245f7 in code_gen_buffer ()
#17 0x55f754cb in cpu_tb_exec (cpu=0x75f730a0,
itb=0x7fffa802b400, tb_exit=0x75f7182c) at ../accel/tcg/cpu-exec.c:357
#18 0x55f76366 in cpu_loop_exec_tb (cpu=0x75f730a0,

Re: [PATCH 3/3] isa/piix4: Resolve global variables

2022-01-30 Thread Philippe Mathieu-Daudé via

On 12/1/22 22:36, Bernhard Beschow wrote:

Now that piix4_set_irq's opaque parameter references own PIIX4State,
piix4_dev becomes redundant and pci_irq_levels can be moved into PIIX4State.

Signed-off-by: Bernhard Beschow 
---
  hw/isa/piix4.c| 22 +-
  include/hw/southbridge/piix.h |  2 --
  2 files changed, 9 insertions(+), 15 deletions(-)


Finally!

Reviewed-by: Philippe Mathieu-Daudé 



Re: [PATCH 2/3] pci: Always pass own DeviceState to pci_map_irq_fn's

2022-01-30 Thread Philippe Mathieu-Daudé via

On 12/1/22 22:36, Bernhard Beschow wrote:

Passing own DeviceState rather than just the IRQs allows for resolving
global variables.

Signed-off-by: Bernhard Beschow 
---
  hw/isa/piix4.c  | 6 +++---
  hw/pci-host/sh_pci.c| 6 +++---
  hw/pci-host/versatile.c | 6 +++---
  hw/ppc/ppc440_pcix.c| 6 +++---
  hw/ppc/ppc4xx_pci.c | 6 +++---
  5 files changed, 15 insertions(+), 15 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 



Re: [PATCH 05/16] hw/arm: allwinner: Don't enable PSCI conduit when booting guest in EL3

2022-01-30 Thread Niek Linnenbank
Hi Peter,



On Thu, Jan 27, 2022 at 4:46 PM Peter Maydell 
wrote:

> Change the allwinner-h3 based board to use the new boot.c
> functionality to allow us to enable psci-conduit only if the guest is
> being booted in EL1 or EL2, so that if the user runs guest EL3
> firmware code our PSCI emulation doesn't get in its way.
>
> To do this we stop setting the psci-conduit property on the CPU
> objects in the SoC code, and instead set the psci_conduit field in
> the arm_boot_info struct to tell the common boot loader code that
> we'd like PSCI if the guest is starting at an EL that it makes sense
> with.
>
> This affects the orangepi-pc board.
>
> This commit leaves the secondary CPUs in the powered-down state if
> the guest is booting at EL3, which is the same behaviour as before
> this commit.  The secondaries can no longer be started by that EL3
> code making a PSCI call but can still be started via the CPU
> Configuration Module registers (which we model in
> hw/misc/allwinner-cpucfg.c).
>
> Signed-off-by: Peter Maydell 
>

While testing your patches on the orangepi-pc machine, I've found that two
acceptance tests BootLinuxConsole.test_arm_orangepi_bionic_20_08 and
BootLinuxConsole.test_arm_orangepi_uboot_netbsd9 of the orangepi-pc board
are no longer passing on current master:

  ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes avocado
--show=app,console run -t machine:orangepi-pc
tests/avocado/boot_linux_console.py
  ...
 (4/5)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08:
-console: U-Boot SPL 2020.04-armbian (Sep 02 2020 - 10:16:13 +0200)
\console: DRAM:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'4-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic_20_08',
'logdi>
 (5/5)
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9:
/console: U-Boot SPL 2020.01+dfsg-1 (Jan 08 2020 - 08:19:44 +)
console: DRAM:
INTERRUPTED: Test interrupted by SIGTERM\nRunner error occurred: Timeout
reached\nOriginal status: ERROR\n{'name':
'5-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9',
'logd>
RESULTS: PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 2 |
CANCEL 0
JOB TIME   : 221.25 s

Bisecting the error I was able to trace it back to commit 5ead62185d
("memory: Make memory_region_is_mapped() succeed when mapped via an alias").
I'll try to find the original thread and respond to that with this
information.

However, with commit 5ead62185d reverted, all tested passed fine:

ARMBIAN_ARTIFACTS_CACHED=yes AVOCADO_ALLOW_LARGE_STORAGE=yes avocado
--show=app,console run -t machine:orangepi-pc
tests/avocado/boot_linux_console.py
...
PASS (16.48 s)
RESULTS: PASS 5 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 |
CANCEL 0
JOB TIME   : 116.63 s

So for the orangepi-pc and allwinner-h3:

Reviewed-by: Niek Linnenbank 
Tested-by: Niek Linnenbank 


> ---
> If anybody knows for definite that the secondaries should be
> powered-down at startup for guest firmware, we can delete the TODO.
>

Looking at the Allwinner H3 datasheet in 4.4.3.7 page 146, it says that
for the CPU1 Status Register the default value indicates at least that its
not in a
wait for interrupt standby mode. And if I look in U-Boot's
arm/arm/cpu/armv7/sunxi/psci.c code
in the psci_cpu_on implementation, there is an explicit 'power on' part
there, suggesting the secondary CPUs
are by default off. So while I don't have any hard proof, these findings
suggest we are modeling the correct behavior
with secondary CPUs by default off.



> The allwinner-cpucfg.c code makes the reset value for the
> REG_CPU*_RST_CTRL registers "CPUX_RESET_RELEASED", which might
> suggest otherwise, but that could easily just be a QEMU error.
> ---
>  hw/arm/allwinner-h3.c | 9 -
>  hw/arm/orangepi.c | 1 +
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> index f9b7ed18711..f54aff6d2d2 100644
> --- a/hw/arm/allwinner-h3.c
> +++ b/hw/arm/allwinner-h3.c
> @@ -235,11 +235,10 @@ static void allwinner_h3_realize(DeviceState *dev,
> Error **errp)
>  /* CPUs */
>  for (i = 0; i < AW_H3_NUM_CPUS; i++) {
>
> -/* Provide Power State Coordination Interface */
> -qdev_prop_set_int32(DEVICE(>cpus[i]), "psci-conduit",
> -QEMU_PSCI_CONDUIT_SMC);
> -
> -/* Disable secondary CPUs */
> +/*
> + * Disable secondary CPUs. TODO: check whether this is what
> + * guest EL3 firmware would really expect.
> + */
>  qdev_prop_set_bit(DEVICE(>cpus[i]), "start-powered-off",
>i > 0);
>
> diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c
> index e7963822367..68fe9182414 100644
> --- a/hw/arm/orangepi.c
> +++ b/hw/arm/orangepi.c
> @@ -105,6 +105,7 @@ static void orangepi_init(MachineState 

Re: [PATCH 1/3] malta: Move PCI interrupt handling from gt64xxx to piix4

2022-01-30 Thread Philippe Mathieu-Daudé via

On 12/1/22 22:36, Bernhard Beschow wrote:

Handling PCI interrupts in piix4 increases cohesion and reduces differences
between piix4 and piix3.

Signed-off-by: Bernhard Beschow 
---
  hw/isa/piix4.c | 58 +++
  hw/mips/gt64xxx_pci.c  | 62 --
  hw/mips/malta.c|  6 +---
  include/hw/mips/mips.h |  2 +-
  4 files changed, 65 insertions(+), 63 deletions(-)


Great!

Reviewed-by: Philippe Mathieu-Daudé 



Re: [PATCH 03/16] hw/arm/boot: Support setting psci-conduit based on guest EL

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

Currently we expect board code to set the psci-conduit property on
CPUs and ensure that secondary CPUs are created with the
start-powered-off property set to false, if the board wishes to use
QEMU's builtin PSCI emulation.  This worked OK for the virt board
where we first wanted to use it, because the virt board directly
creates its CPUs and is in a reasonable position to set those
properties.  For other boards which model real hardware and use a
separate SoC object, however, it is more awkward.  Most PSCI-using
boards just set the psci-conduit board unconditionally.

This was never strictly speaking correct (because you would not be
able to run EL3 guest firmware that itself provided the PSCI
interface, as the QEMU implementation would overrule it), but mostly
worked in practice because for non-PSCI SMC calls QEMU would emulate
the SMC instruction as normal (by trapping to guest EL3).  However,
we would like to make our PSCI emulation follow the part of the SMCC
specification that mandates that SMC calls with unknown function
identifiers return a failure code, which means that all SMC calls
will be handled by the PSCI code and the "emulate as normal" path
will no longer be taken.

We tried to implement that in commit 9fcd15b9193e81
("arm: tcg: Adhere to SMCCC 1.3 section 5.2"), but this
regressed attempts to run EL3 guest code on the affected boards:
  * mcimx6ul-evk, mcimx7d-sabre, orangepi, xlnx-zcu102
  * for the case only of EL3 code loaded via -kernel (and
not via -bios or -pflash), virt and xlnx-versal-virt
so for the 7.0 release we reverted it (in commit 4825eaae4fdd56f).

This commit provides a mechanism that boards can use to arrange that
psci-conduit is set if running guest code at a low enough EL but not
if it would be running at the same EL that the conduit implies that
the QEMU PSCI implementation is using.  (Later commits will convert
individual board models to use this mechanism.)

We do this by moving the setting of the psci-conduit and
start-powered-off properties to arm_load_kernel().  Boards which want
to potentially use emulated PSCI must set a psci_conduit field in the
arm_boot_info struct to the type of conduit they want to use (SMC or
HVC); arm_load_kernel() will then set the CPUs up accordingly if it
is not going to start the guest code at the same or higher EL as the
fake QEMU firmware would be at.

Board/SoC code which uses this mechanism should no longer set the CPU
psci-conduit property directly.  It should only set the
start-powered-off property for secondaries if EL3 guest firmware
running bare metal expects that rather than the alternative "all CPUs
start executing the firmware at once".

Note that when calculating whether we are going to run guest
code at EL3, we ignore the setting of arm_boot_info::secure_board_setup,
which might cause us to run a stub bit of guest code at EL3 which
does some board-specific setup before dropping to EL2 or EL1 to
run the guest kernel. This is OK because only one board that
enables PSCI sets secure_board_setup (the highbank board), and
the stub code it writes will behave the same way whether the
one SMC call it makes is handled by "emulate the SMC" or by
"PSCI default returns an error code". So we can leave that stub
code in place until after we've changed the PSCI default behaviour;
at that point we will remove it.

Signed-off-by: Peter Maydell 
---
  include/hw/arm/boot.h | 10 +
  hw/arm/boot.c | 50 +++
  2 files changed, 60 insertions(+)


Reviewed-by: Richard Henderson 


r~



Re: [PATCH 02/16] cpu.c: Make start-powered-off settable after realize

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

The CPU object's start-powered-off property is currently only
settable before the CPU object is realized.  For arm machines this is
awkward, because we would like to decide whether the CPU should be
powered-off based on how we are booting the guest code, which is
something done in the machine model code and in common code called by
the machine model, which runs much later and in completely different
parts of the codebase from the SoC object code that is responsible
for creating and realizing the CPU objects.

Allow start-powered-off to be set after realize.  Since this isn't
something that's supported by the DEFINE_PROP_* macros, we have to
switch the property definition to use the
object_class_property_add_bool() function.

Note that it doesn't conceptually make sense to change the setting of
the property after the machine has been completely initialized,
beacuse this would mean that the behaviour of the machine when first
started would differ from its behaviour when the system is
subsequently reset.  (It would also require the underlying state to
be migrated, which we don't do.)

Signed-off-by: Peter Maydell
---
  cpu.c | 22 +-
  1 file changed, 21 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v3 36/40] bsd-user/signal.c: implement do_sigaction

2022-01-30 Thread Warner Losh
On Sun, Jan 30, 2022 at 2:19 PM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 1/29/22 10:28, Warner Losh wrote:
> > +if (block_signals()) {
> > +return -TARGET_ERESTART;
> > +}
> > +
> > +k = _table[sig - 1];
> > +if (oact) {
> > +oact->_sa_handler = tswapal(k->_sa_handler);
> > +oact->sa_flags = tswap32(k->sa_flags);
> > +oact->sa_mask = k->sa_mask;
> > +}
> > +if (act) {
> > +/* XXX: this is most likely not threadsafe. */
>
> It surely is -- we never set another thread's sigaction, and we've just
> blocked all
> signals, so we're signal-safe.  Am I missing something?
>

Now that I look at it, I can't understand why this comment is here. I'll
remove it.

Warner


> Otherwise,
> Reviewed-by: Richard Henderson 
>
>
> r~
>


Re: [PATCH 01/16] target/arm: make psci-conduit settable after realize

2022-01-30 Thread Richard Henderson

On 1/28/22 02:46, Peter Maydell wrote:

We want to allow the psci-conduit property to be set after realize,
because the parts of the code which are best placed to decide if it's
OK to enable QEMU's builtin PSCI emulation (the board code and the
arm_load_kernel() function are distant from the code which creates
and realizes CPUs (typically inside an SoC object's init and realize
method) and run afterwards.

Since the DEFINE_PROP_* macros don't have support for creating
properties which can be changed after realize, change the property to
be created with object_property_add_uint32_ptr(), which is what we
already use in this function for creating settable-after-realize
properties like init-svtor and init-nsvtor.

Note that it doesn't conceptually make sense to change the setting of
the property after the machine has been completely initialized,
beacuse this would mean that the behaviour of the machine when first
started would differ from its behaviour when the system is
subsequently reset.  (It would also require the underlying state to
be migrated, which we don't do.)

Signed-off-by: Peter Maydell
---
  target/arm/cpu.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v3 36/40] bsd-user/signal.c: implement do_sigaction

2022-01-30 Thread Richard Henderson

On 1/29/22 10:28, Warner Losh wrote:

+if (block_signals()) {
+return -TARGET_ERESTART;
+}
+
+k = _table[sig - 1];
+if (oact) {
+oact->_sa_handler = tswapal(k->_sa_handler);
+oact->sa_flags = tswap32(k->sa_flags);
+oact->sa_mask = k->sa_mask;
+}
+if (act) {
+/* XXX: this is most likely not threadsafe. */


It surely is -- we never set another thread's sigaction, and we've just blocked all 
signals, so we're signal-safe.  Am I missing something?


Otherwise,
Reviewed-by: Richard Henderson 


r~



Re: [PATCH v3 35/40] bsd-user/signal.c: implement do_sigreturn

2022-01-30 Thread Richard Henderson

On 1/29/22 10:28, Warner Losh wrote:

+static int reset_signal_mask(target_ucontext_t *ucontext)
+{
+int i;
+sigset_t blocked;
+target_sigset_t target_set;
+TaskState *ts = (TaskState *)thread_cpu->opaque;
+
+for (i = 0; i < TARGET_NSIG_WORDS; i++)
+if (__get_user(target_set.__bits[i],
+>uc_sigmask.__bits[i])) {
+return -TARGET_EFAULT;
+}


Nit: missing braces for the loop.  Otherwise,

Reviewed-by: Richard Henderson 


r~



Re: [PATCH v3 7/7] target/riscv: add a MAINTAINERS entry for XVentanaCondOps

2022-01-30 Thread Richard Henderson

On 1/29/22 01:56, Philipp Tomsich wrote:

The XVentanaCondOps extension is supported by VRULL on behalf of the
Ventana Micro.  Add myself as a point-of-contact.

Signed-off-by: Philipp Tomsich

---


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v3 6/7] target/riscv: Add XVentanaCondOps custom extension

2022-01-30 Thread Richard Henderson

On 1/29/22 01:56, Philipp Tomsich wrote:

This adds the decoder and translation for the XVentanaCondOps custom
extension (vendor-defined by Ventana Micro Systems), which is
documented 
athttps://github.com/ventanamicro/ventana-custom-extensions/releases/download/v1.0.0/ventana-custom-extensions-v1.0.0.pdf

This commit then also adds a guard-function (has_XVentanaCondOps_p)
and the decoder function to the table of decoders, enabling the
support for the XVentanaCondOps extension.

Signed-off-by: Philipp Tomsich

---


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v3 5/7] target/riscv: iterate over a table of decoders

2022-01-30 Thread Richard Henderson

On 1/29/22 01:56, Philipp Tomsich wrote:

-if (!decode_insn16(ctx, opcode)) {
-gen_exception_illegal(ctx);
-}
+if (decode_insn16(ctx, opcode))
+return;

...

-if (!decode_insn32(ctx, opcode32)) {
-gen_exception_illegal(ctx);
+
+for (size_t i = 0; i < ARRAY_SIZE(decoders); ++i) {
+if (!decoders[i].guard_func(ctx))
+continue;
+
+if (decoders[i].decode_func(ctx, opcode32))
+return;


Missing braces, per style.  Otherwise,

Reviewed-by: Richard Henderson 

r~



Re: [PATCH v3 4/7] target/riscv: access cfg structure through DisasContext

2022-01-30 Thread Richard Henderson

On 1/29/22 01:56, Philipp Tomsich wrote:

The Zb[abcs] support code still uses the RISCV_CPU macros to access
the configuration information (i.e., check whether an extension is
available/enabled).  Now that we provide this information directly
from DisasContext, we can access this directly via the cfg_ptr field.

Signed-off-by: Philipp Tomsich
Suggested-by: Richard Henderson

---

Changes in v3:
- (new patch) change Zb[abcs] implementation to use cfg_ptr (copied
   into DisasContext) instead of going throuhg RISCV_CPU

  target/riscv/insn_trans/trans_rvb.c.inc | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v3 3/7] target/riscv: access configuration through cfg_ptr in DisasContext

2022-01-30 Thread Richard Henderson

On 1/29/22 01:56, Philipp Tomsich wrote:

The implementation in trans_{rvi,rvv,rvzfh}.c.inc accesses the shallow
copies (in DisasContext) of some of the elements available in the
RISCVCPUConfig structure.  This commit redirects accesses to use the
cfg_ptr copied into DisasContext and removes the shallow copies.

Signed-off-by: Philipp Tomsich
Suggested-by: Richard Henderson

---

Changes in v3:
- (new patch) test extension-availability through cfg_ptr in
   DisasContext, removing the fields that have been copied into
   DisasContext directly

  target/riscv/insn_trans/trans_rvi.c.inc   |   2 +-
  target/riscv/insn_trans/trans_rvv.c.inc   | 104 +++---
  target/riscv/insn_trans/trans_rvzfh.c.inc |   4 +-
  target/riscv/translate.c  |  14 ---
  4 files changed, 55 insertions(+), 69 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v3 2/7] target/riscv: riscv_tr_init_disas_context: copy pointer-to-cfg into cfg_ptr

2022-01-30 Thread Richard Henderson

On 1/29/22 01:56, Philipp Tomsich wrote:

As the number of extensions is growing, copying them individiually
into the DisasContext will scale less and less... instead we populate
a pointer to the RISCVCPUConfig structure in the DisasContext.

This adds an extra indirection when checking for the availability of
an extension (compared to copying the fields into DisasContext).
While not a performance problem today, we can always (shallow) copy
the entire structure into the DisasContext (instead of putting a
pointer to it) if this is ever deemed necessary.

Signed-off-by: Philipp Tomsich
Suggested-by: Richard Henderson

---

Changes in v3:
- (new patch) copy pointer to element cfg into DisasContext

  target/riscv/translate.c | 2 ++
  1 file changed, 2 insertions(+)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v3 1/7] target/riscv: refactor (anonymous struct) RISCVCPU.cfg into 'struct RISCVCPUConfig'

2022-01-30 Thread Richard Henderson

On 1/29/22 01:56, Philipp Tomsich wrote:

+struct RISCVCPUConfig cfg;


Coding style says use a typedef for the struct.

Otherwise,
Reviewed-by: Richard Henderson 


r~



Re: [PATCH] hw/ppc/vof: Add missing includes

2022-01-30 Thread Philippe Mathieu-Daudé via

Cc'ing qemu-trivial@

On 22/1/22 01:31, Philippe Mathieu-Daudé wrote:

vof.h requires "qom/object.h" for DECLARE_CLASS_CHECKERS(),
"exec/memory.h" for address_space_read/write(),
"exec/address-spaces.h" for address_space_memory
and more importantly "cpu.h" for target_ulong.

vof.c doesn't need "exec/ram_addr.h".

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/ppc/vof.c | 1 -
  include/hw/ppc/vof.h | 5 +
  2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/vof.c b/hw/ppc/vof.c
index 73adc44ec2..2b63a62875 100644
--- a/hw/ppc/vof.c
+++ b/hw/ppc/vof.c
@@ -16,7 +16,6 @@
  #include "qemu/units.h"
  #include "qemu/log.h"
  #include "qapi/error.h"
-#include "exec/ram_addr.h"
  #include "exec/address-spaces.h"
  #include "hw/ppc/vof.h"
  #include "hw/ppc/fdt.h"
diff --git a/include/hw/ppc/vof.h b/include/hw/ppc/vof.h
index 97fdef758b..f8c0effcaf 100644
--- a/include/hw/ppc/vof.h
+++ b/include/hw/ppc/vof.h
@@ -6,6 +6,11 @@
  #ifndef HW_VOF_H
  #define HW_VOF_H
  
+#include "qom/object.h"

+#include "exec/address-spaces.h"
+#include "exec/memory.h"
+#include "cpu.h"
+
  typedef struct Vof {
  uint64_t top_addr; /* copied from rma_size */
  GArray *claimed; /* array of SpaprOfClaimed */





Re: should we have a Kconfig "device group" for I2C devices?

2022-01-30 Thread Philippe Mathieu-Daudé via

+Alex

On 28/1/22 15:30, Paolo Bonzini wrote:

On 1/28/22 15:17, Peter Maydell wrote:

Hi; I've been looking into what is the right way to handle in Kconfig
an i2c device which is intended for the user to specify on the command
line with a -device option.
(It's the lsm303dlhc magnetometer, currently in code review:
https://patchew.org/QEMU/20210921093227.18592-1-kevin.towns...@linaro.org/  
)


Currently all our i2c devices are just pulled in by "select FOO" from
the Kconfig stanza for a board which has that kind of sensor hardwired
on-board. But for at least some of them it works fine to just specify
them on the commandline of any board that has an i2c controller that
allows pluggable devices. (For instance we do that kind of commandline
plugging in our test suite with tests/qtest/tmp105-test.c.)

What's the best way to structure this? For PCI we have the "device
group" PCI_DEVICES as documented in
https://qemu-project.gitlab.io/qemu/devel/kconfig.html#guidelines-for-writing-kconfig-files 


and PCI devices say
 default y if PCI_DEVICES
 depends on PCI

For ISA devices we seem to make them say
 default y
 depends on ISA_BUS

I2C devices currently just say
 depends on I2C

Should we have an I2C_DEVICES, which boards where there's a sensible
user-pluggable i2c controller can specifically select ? Or should we
mark the i2c devices which are sensibly user-pluggable as
"default y" ? Or something else ?


Yes, I think it's a good idea to have I2C_DEVICES like we have 
PCI_DEVICES.  This way we can skip them on x86 (where the SMBus 
controller is mostly a legacy device) but include them by default on 
AVR, embedded ARM, etc.


My first reaction was "Yes, generically each bus should have its
bus_DEVICES" switch; but then I wondered in which use case we still
need this switch.

- In the default case (--with-default-devices) if a board exposes a bus,
  we want to have all the devices compatible with the bus to be built.

- If we use --without-default-devices, then we only want the explicitly
  listed devices, and PCI_DEVICES=y here seems dubious to me.

- If we use --with-devices, this is similar to the previous case
  (--without-default-devices is a specific --with-devices case [*]).

Paolo, in what case do you see having a ${bus}_DEVICES config useful?

Thanks,

Phil.

[*] Actually I think --with-devices replaced
--with[out]-default-devices, it is more powerful / customizable;
we should only keep / maintain --with-devices and drop
--with[out]-default-devices options.



Re: [PATCH] qapi/block: Cosmetic change in BlockExportType schema

2022-01-30 Thread Philippe Mathieu-Daudé via

On 28/1/22 21:54, Eric Blake wrote:

On Wed, Jan 19, 2022 at 01:14:39PM +0100, Philippe Mathieu-Daudé wrote:

From: Philippe Mathieu-Daude 

From: Philippe Mathieu-Daudé 


The doubled From: looks odd here.  I'll double-check that git doesn't
mess up the actual commit once I apply the patch.


I played with the git --from option to not appear in the list as
'"Philippe Mathieu-Daudé via" ':
https://lore.kernel.org/qemu-devel/efc5f304-f3d2-ff7b-99a6-673595ff0...@amsat.org/
by using a different sendemail.from (removing the acute in my
lastname) to force a correct author.from.
git-am should have picked the 2nd form, but I see the 1st in commit
3a8fa0edd1. Just curious, did you had to modify it manually?

Anyway, thanks for merging this :)



Fix long line introduced in commit bb01ea73110 ("qapi/block:
Restrict vhost-user-blk to CONFIG_VHOST_USER_BLK_SERVER").

Suggested-by: Markus Armbruster 
Acked-by: Markus Armbruster 
Signed-off-by: Philippe Mathieu-Daudé 
---
  qapi/block-export.json | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)




Re: build-oss-fuzz CI job often times out

2022-01-30 Thread Philippe Mathieu-Daudé via

Cc'ing Alex/Darren.

On 29/1/22 14:34, Peter Maydell wrote:

Hi; the build-oss-fuzz gitlab CI job seems to intermittently
but quite commonly hit the 1 hour timeout mark and get killed.
Examples from the last couple of days:

https://gitlab.com/qemu-project/qemu/-/jobs/2030815488
https://gitlab.com/qemu-project/qemu/-/jobs/2029246068
https://gitlab.com/qemu-project/qemu/-/jobs/2029013479
https://gitlab.com/qemu-project/qemu/-/jobs/2024871970
https://gitlab.com/qemu-project/qemu/-/jobs/2022584981

Can we do anything to cut down on the runtime of this job
and make it reliably complete inside the time limit?

thanks
-- PMM





Re: [PATCH v2 0/8] target/ppc: powerpc_excp improvements [74xx] (5/n)

2022-01-30 Thread Cédric Le Goater

On 1/27/22 21:11, Fabiano Rosas wrote:

Changes from v1:

- Restored the 'sc 1' support to avoid breaking the pegasos2 machine.

I tested this version in the G4 with the following OSes:

- Linux 5.15 (5.16 seems to be broken, even with master)



Have you tried pmac32 defconfig plus these configs :

CONFIG_SERIAL_PMACZILOG=y
CONFIG_SERIAL_PMACZILOG_TTYS=y
CONFIG_SERIAL_PMACZILOG_CONSOLE=y
CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y

5.17-rc1 looks fine with your patchset.

Thanks,

C.



- FreeBSD 13
- Mac OS 9.2
- Mac OS Darwin 6.0
- Mac OS X 10.4
- MorphOS 3.15 (-M pegasos2 and -M mac99,via=pmu)

Based on legoater/ppc-7.0

v1:
https://lore.kernel.org/r/20220126164200.1048677-1-faro...@linux.ibm.com

Fabiano Rosas (8):
   target/ppc: Introduce powerpc_excp_74xx
   target/ppc: Simplify powerpc_excp_74xx
   target/ppc: 74xx: Machine Check exception cleanup
   target/ppc: 74xx: External interrupt cleanup
   target/ppc: 74xx: Program exception cleanup
   target/ppc: 74xx: System Call exception cleanup
   target/ppc: 74xx: System Reset interrupt cleanup
   target/ppc: 74xx: Set SRRs directly in exception code

  target/ppc/excp_helper.c | 197 +++
  1 file changed, 197 insertions(+)






[PATCH v2] target/ppc: Remove support for the PowerPC 602 CPU

2022-01-30 Thread Cédric Le Goater
The 602 was derived from the PowerPC 603, for the gaming market it
seems. It was hardly used and no firmware supporting the CPU could be
found. Drop support.

Cc: Fabiano Rosas 
Cc: Víctor Colombo 
Signed-off-by: Cédric Le Goater 
---

 v2: - Fixed PPC_602_SPEC compile issue (Victor)
 - Dropped MSR_AP and MSR_SA (Fabiano)

 target/ppc/cpu-models.h  |   1 -
 target/ppc/cpu.h |   8 +-
 target/ppc/helper.h  |   1 -
 target/ppc/cpu-models.c  |   2 -
 target/ppc/cpu_init.c| 147 ---
 target/ppc/excp_helper.c |   1 -
 target/ppc/int_helper.c  |  21 -
 target/ppc/mfrom_table_gen.c |  34 
 target/ppc/translate.c   |  30 ---
 target/ppc/mfrom_table.c.inc |  78 ---
 10 files changed, 1 insertion(+), 322 deletions(-)
 delete mode 100644 target/ppc/mfrom_table_gen.c
 delete mode 100644 target/ppc/mfrom_table.c.inc

diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
index bf1dc7e5ca3d..612978a3fbd2 100644
--- a/target/ppc/cpu-models.h
+++ b/target/ppc/cpu-models.h
@@ -208,7 +208,6 @@ enum {
 CPU_POWERPC_601_v0 = 0x00010001,
 CPU_POWERPC_601_v1 = 0x00010001,
 CPU_POWERPC_601_v2 = 0x00010002,
-CPU_POWERPC_602= 0x00050100,
 CPU_POWERPC_603= 0x00030100,
 CPU_POWERPC_603E_v11   = 0x00060101,
 CPU_POWERPC_603E_v12   = 0x00060102,
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 66e13075c3df..dcd83b503c62 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -321,9 +321,7 @@ typedef enum {
 #define MSR_UCLE 26 /* User-mode cache lock enable for BookE */
 #define MSR_VR   25 /* altivec availablex hflags */
 #define MSR_SPE  25 /* SPE enable for BookE x hflags */
-#define MSR_AP   23 /* Access privilege state on 602  hflags */
 #define MSR_VSX  23 /* Vector Scalar Extension (ISA 2.06 and later) x hflags */
-#define MSR_SA   22 /* Supervisor access mode on 602  hflags */
 #define MSR_S22 /* Secure state  */
 #define MSR_KEY  19 /* key bit on 603e   */
 #define MSR_POW  18 /* Power management  */
@@ -477,9 +475,7 @@ typedef enum {
 #define msr_ucle ((env->msr >> MSR_UCLE) & 1)
 #define msr_vr   ((env->msr >> MSR_VR)   & 1)
 #define msr_spe  ((env->msr >> MSR_SPE)  & 1)
-#define msr_ap   ((env->msr >> MSR_AP)   & 1)
 #define msr_vsx  ((env->msr >> MSR_VSX)  & 1)
-#define msr_sa   ((env->msr >> MSR_SA)   & 1)
 #define msr_key  ((env->msr >> MSR_KEY)  & 1)
 #define msr_pow  ((env->msr >> MSR_POW)  & 1)
 #define msr_tgpr ((env->msr >> MSR_TGPR) & 1)
@@ -2142,8 +2138,6 @@ enum {
 PPC_MFTB   = 0x0200ULL,
 
 /* Fixed-point unit extensions   */
-/*   PowerPC 602 specific*/
-PPC_602_SPEC   = 0x0400ULL,
 /*   isel instruction*/
 PPC_ISEL   = 0x0800ULL,
 /*   popcntb instruction */
@@ -2245,7 +2239,7 @@ enum {
 #define PPC_TCG_INSNS  (PPC_INSNS_BASE | PPC_POWER | PPC_POWER2 \
 | PPC_POWER_RTC | PPC_POWER_BR | PPC_64B \
 | PPC_64BX | PPC_64H | PPC_WAIT | PPC_MFTB \
-| PPC_602_SPEC | PPC_ISEL | PPC_POPCNTB \
+| PPC_ISEL | PPC_POPCNTB \
 | PPC_STRING | PPC_FLOAT | PPC_FLOAT_EXT \
 | PPC_FLOAT_FSQRT | PPC_FLOAT_FRES \
 | PPC_FLOAT_FRSQRTE | PPC_FLOAT_FRSQRTES \
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index bdbbd5e1d90f..f2e5060910de 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -646,7 +646,6 @@ DEF_HELPER_FLAGS_2(slbieg, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl)
 DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
 
-DEF_HELPER_FLAGS_1(602_mfrom, TCG_CALL_NO_RWG_SE, tl, tl)
 DEF_HELPER_1(msgsnd, void, tl)
 DEF_HELPER_2(msgclr, void, env, tl)
 DEF_HELPER_1(book3s_msgsnd, void, tl)
diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
index 764afe5a2afb..a2c720cc4d0c 100644
--- a/target/ppc/cpu-models.c
+++ b/target/ppc/cpu-models.c
@@ -428,8 +428,6 @@
 "PowerPC 601v1")
 POWERPC_DEF("601_v2",CPU_POWERPC_601_v2, 601v,
 "PowerPC 601v2")
-POWERPC_DEF("602",   CPU_POWERPC_602,602,
-"PowerPC 602")
 POWERPC_DEF("603",   CPU_POWERPC_603,603,
 "PowerPC 603")
 POWERPC_DEF("603e_v1.1", CPU_POWERPC_603E_v11,   

Re: [PULL 0/4] NBD patches for 2022-01-28

2022-01-30 Thread Peter Maydell
On Fri, 28 Jan 2022 at 23:04, Eric Blake  wrote:
>
> The following changes since commit 7a1043cef91739ff4b59812d30f1ed2850d3d34e:
>
>   Merge remote-tracking branch 'remotes/bonzini-gitlab/tags/for-upstream' 
> into staging (2022-01-28 14:04:01 +)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2022-01-28
>
> for you to fetch changes up to 6384dd534d742123d26c008d9794b20bc41359d5:
>
>   iotests/block-status-cache: New test (2022-01-28 16:55:23 -0600)
>
> 
> nbd patches for 2022-01-28
>
> - Hanna Reitz: regression fix for block status caching
> - Philippe Mathieu-Daude: documentation formatting
> - Nir Soffer: dead code removal


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM



Re: [PATCH 00/16] arm: Fix handling of unrecognized functions in PSCI emulation

2022-01-30 Thread Edgar E. Iglesias
On Thu, Jan 27, 2022 at 4:46 PM Peter Maydell 
wrote:

> This series fixes our handling of PSCI calls where the function ID is
> not recognized. These are supposed to return an error value, but
> currently we instead emulate the SMC or HVC instruction to trap to the
> guest at EL3 or EL2. Particularly of note for code review:
>  * patches 4-9 include some "is this the right behaviour for
>this hardware" questions for the maintainers of those boards
>  * patch 15 has a DTB API question, as well as being a change in
>what we edit in a DTB we are passed by the user
>  * testing of the affected machines would be welcome
>
> We tried to fix that bug in commit 9fcd15b9193e819b, but ran into
> regressions and so reverted it in commit 4825eaae4fdd56fba0f for
> release 7.0.  This series fixes the underlying problems causing the
> regressions and reverts the revert.  It also fixes some other bugs
> that were preventing booting of guests on the midway board and that
> meant that the Linux kernel I tested couldn't bring up the secondary
> CPUs when running with more than one guest CPU.
>
> The regressions happened on boards which enabled PSCI emulation while
> still running guest code at EL3. This used to work (as long as the
> guest code itself wasn't trying to implement PSCI!)  because of the
> fall-through-to-emulate-the-insn behaviour, but once the PSCI
> emulation handles all SMC calls, most EL3 guest code will stop working
> correctly. The solution this patchset adopts is to avoid enabling
> QEMU's PSCI emulation when running guest code at EL3.
>
> The affected boards are:
>  * orangepi-pc, mcimx6ul-evk, mcimx7d-sabre, highbank, midway,
>xlnx-zcu102 (for any EL3 guest code)
>  * xlnx-versal-virt (only for EL3 code run via -kernel)
>  * virt (only for EL3 code run via -kernel or generic-loader)
> For all these cases we will no longer enable PSCI emulation.
> (This might in theory break guest code that used to work because
> it was relying on running under QEMU and having the PSCI emulation
> despite being at EL3 itself, but hopefully such code is rare.)
>
> In order to only enable PSCI emulation when the guest is running at an
> exception level lower than the level that our PSCI emulation
> "firmware" would be running at, we make the arm_load_kernel() code in
> boot.c responsible for setting the CPU properties psci-conduit and
> start-powered-off. This is because only that code knows what EL it is
> going to start the guest at (which depends on things like whether it
> has decided that the guest is a Linux kernel or not).
>
> The complicated case in all of this is the highbank and midway board
> models, because of all the boards which enable QEMU's PSCI emulation
> only these also use the boot.c secure_board_setup flag to say "run a
> fragment of QEMU-provided boot code in the guest at EL3 to set
> something up before running the guest at EL2 or EL1". That fragment of
> code includes use of the SMC instruction, so when PSCI emulation
> starts making that a NOP rather than a trap-to-guest-EL3 the setup
> code will change behaviour. Fortunately, for this specific board's use
> case the NOP is fine. The purpose of the setup code is to arrange that
> unknown SMCs act as NOPs, because guest code may use a
> highbank/midway-specific SMC ABI to enable the L2x0 cache
> controller. So when the PSCI emulation starts to NOP the unknown SMCs
> the setup code won't actively break, and the guest behaviour will
> still be OK. (See patch 11's commit message for fuller details.)
>
> Patches 1 and 2 make the relevant CPU properties settable after the
> CPU object has been realized. This is necessary because
> arm_load_kernel() runs very late, after the whole machine (including
> the CPU objects) has been fully initialized.  (It was the restriction
> on setting these properties before realize that previously led us to
> set them in the SoC emulation code and thus to do it unconditionally.)
>
> Patch 3 provides the "set up psci conduit" functionality in the boot.c
> code; this is opt-in per board by setting a field in the arm_boot_info
> struct.
>
> Patches 4 to 9 move the individual boards across to using the new
> approach. In each case I had to make a decision about the behaviour of
> secondary CPUs when running guest firmware at EL3 -- should the
> secondaries start off powered-down (waiting for the guest to power
> them up via some kind of emulated power-control device), or powered-up
> (so they all start running the firmware at once)? In a few cases I was
> able to find the answer to this; in the rest I have erred on the side
> of retaining the current "start powered down" behaviour, and added a
> TODO comment to that effect. If you know the actual hardware
> behaviour, let me know.
>

Hi Peter,

For ZynqMP and Versal, they should start up powered-off.



>
> Patch 10 is the revert-the-revert patch.
>
> Patch 11 removes the highbank/midway board use of the secure_board_setup
> functionality; the commit message has 

[PATCH v2] target/i386: Add kvm_get_one_msr helper

2022-01-30 Thread Yang Weijiang
When try to get one msr from KVM, I found there's no such kind of
existing interface while kvm_put_one_msr() is there. So here comes
the patch. It'll remove redundant preparation code before finally
call KVM_GET_MSRS IOCTL.

No functional change intended.

v2:
 Per Paolo's suggestion, move the helper before uses to eliminate
 a forward declaration.

base-commit: 48302d4eb628ff0bea4d7e92cbf6b726410eb4c3

Signed-off-by: Yang Weijiang 
---
 target/i386/kvm/kvm.c | 48 ---
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 2c8feb4a6f..627535395a 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -201,32 +201,45 @@ bool kvm_hv_vpindex_settable(void)
 return hv_vpindex_settable;
 }
 
-static int kvm_get_tsc(CPUState *cs)
+static int kvm_get_one_msr(X86CPU *cpu, int index, uint64_t *value)
 {
-X86CPU *cpu = X86_CPU(cs);
-CPUX86State *env = >env;
+int ret;
 struct {
 struct kvm_msrs info;
 struct kvm_msr_entry entries[1];
-} msr_data = {};
+} msr_data = {
+.info.nmsrs = 1,
+.entries[0].index = index,
+};
+
+ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, _data);
+if (ret < 0) {
+return ret;
+}
+assert(ret == 1);
+*value = msr_data.entries[0].data;
+return ret;
+}
+
+static int kvm_get_tsc(CPUState *cs)
+{
+X86CPU *cpu = X86_CPU(cs);
+CPUX86State *env = >env;
+uint64_t value;
 int ret;
 
 if (env->tsc_valid) {
 return 0;
 }
 
-memset(_data, 0, sizeof(msr_data));
-msr_data.info.nmsrs = 1;
-msr_data.entries[0].index = MSR_IA32_TSC;
 env->tsc_valid = !runstate_is_running();
 
-ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, _data);
+ret = kvm_get_one_msr(cpu, MSR_IA32_TSC, );
 if (ret < 0) {
 return ret;
 }
 
-assert(ret == 1);
-env->tsc = msr_data.entries[0].data;
+env->tsc = value;
 return 0;
 }
 
@@ -1478,21 +1491,14 @@ static int hyperv_init_vcpu(X86CPU *cpu)
  * the kernel doesn't support setting vp_index; assert that its value
  * is in sync
  */
-struct {
-struct kvm_msrs info;
-struct kvm_msr_entry entries[1];
-} msr_data = {
-.info.nmsrs = 1,
-.entries[0].index = HV_X64_MSR_VP_INDEX,
-};
-
-ret = kvm_vcpu_ioctl(cs, KVM_GET_MSRS, _data);
+uint64_t value;
+
+ret = kvm_get_one_msr(cpu, HV_X64_MSR_VP_INDEX, );
 if (ret < 0) {
 return ret;
 }
-assert(ret == 1);
 
-if (msr_data.entries[0].data != hyperv_vp_index(CPU(cpu))) {
+if (value != hyperv_vp_index(CPU(cpu))) {
 error_report("kernel's vp_index != QEMU's vp_index");
 return -ENXIO;
 }
-- 
2.27.0




[PATCH v1 0/1] hw/arm: versal-virt: Always call arm_load_kernel()

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

This should be applied on top of Peter Maydell's
"arm: Fix handling of unrecognized functions in PSCI emulation"
patch series.

This fixes an issue reported by Peter Maydell. We should
always call arm_load_kernel() regardless of kernel_filename being
set. This is needed because arm_load_kernel() sets up reset for
the CPUs.

Cheers,
Edgar

Edgar E. Iglesias (1):
  hw/arm: versal-virt: Always call arm_load_kernel()

 hw/arm/xlnx-versal-virt.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

-- 
2.25.1




[PATCH v1 1/1] hw/arm: versal-virt: Always call arm_load_kernel()

2022-01-30 Thread Edgar E. Iglesias
From: "Edgar E. Iglesias" 

Always call arm_load_kernel() regardless of kernel_filename being
set. This is needed because arm_load_kernel() sets up reset for
the CPUs.

Fixes: 6f16da53ff (hw/arm: versal: Add a virtual Xilinx Versal board)
Reported-by: Peter Maydell 
Signed-off-by: Edgar E. Iglesias 
---
 hw/arm/xlnx-versal-virt.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index ef3231cdbb..7c7baff8b7 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -681,20 +681,13 @@ static void versal_virt_init(MachineState *machine)
 s->binfo.get_dtb = versal_virt_get_dtb;
 s->binfo.modify_dtb = versal_virt_modify_dtb;
 s->binfo.psci_conduit = psci_conduit;
-if (machine->kernel_filename) {
-arm_load_kernel(>soc.fpd.apu.cpu[0], machine, >binfo);
-} else {
-AddressSpace *as = arm_boot_address_space(>soc.fpd.apu.cpu[0],
-  >binfo);
+if (!machine->kernel_filename) {
 /* Some boot-loaders (e.g u-boot) don't like blobs at address 0 (NULL).
  * Offset things by 4K.  */
 s->binfo.loader_start = 0x1000;
 s->binfo.dtb_limit = 0x100;
-if (arm_load_dtb(s->binfo.loader_start,
- >binfo, s->binfo.dtb_limit, as, machine) < 0) {
-exit(EXIT_FAILURE);
-}
 }
+arm_load_kernel(>soc.fpd.apu.cpu[0], machine, >binfo);
 
 for (i = 0; i < XLNX_VERSAL_NUM_OSPI_FLASH; i++) {
 BusState *spi_bus;
-- 
2.25.1




Re: [Qemu-devel] [PULL 10/10] hw/arm: versal: Add a virtual Xilinx Versal board

2022-01-30 Thread Edgar E. Iglesias
On Thu, Jan 27, 2022 at 2:10 PM Peter Maydell 
wrote:

> On Fri, 2 Nov 2018 at 17:24, Peter Maydell 
> wrote:
> >
> > From: "Edgar E. Iglesias" 
> >
> > Add a virtual Xilinx Versal board.
> >
> > This board is based on the Xilinx Versal SoC. The exact
> > details of what peripherals are attached to this board
> > will remain in control of QEMU. QEMU will generate an
> > FDT on the fly for Linux and other software to auto-discover
> > peripherals.
>
> Hi Edgar; I was just looking at the Versal board code for
> something else I was working on, and I noticed a bug that's been
> there since it was added in this patch:
>
> > +s->binfo.ram_size = machine->ram_size;
> > +s->binfo.kernel_filename = machine->kernel_filename;
> > +s->binfo.kernel_cmdline = machine->kernel_cmdline;
> > +s->binfo.initrd_filename = machine->initrd_filename;
> > +s->binfo.loader_start = 0x0;
> > +s->binfo.get_dtb = versal_virt_get_dtb;
> > +s->binfo.modify_dtb = versal_virt_modify_dtb;
> > +if (machine->kernel_filename) {
> > +arm_load_kernel(s->soc.fpd.apu.cpu[0], >binfo);
> > +} else {
> > +AddressSpace *as = arm_boot_address_space(s->soc.fpd.apu.cpu[0],
> > +  >binfo);
> > +/* Some boot-loaders (e.g u-boot) don't like blobs at address 0
> (NULL).
> > + * Offset things by 4K.  */
> > +s->binfo.loader_start = 0x1000;
> > +s->binfo.dtb_limit = 0x100;
> > +if (arm_load_dtb(s->binfo.loader_start,
> > + >binfo, s->binfo.dtb_limit, as) < 0) {
> > +exit(EXIT_FAILURE);
> > +}
> > +}
>
> The board init code only calls arm_load_kernel() if
> machine->kernel_filename
> is set. This is a bug, because calling arm_load_kernel() is mandatory
> for board code -- it is the place where we set up the reset handler
> that resets the CPUs, so if you don't call it the CPU objects don't
> get reset.
>
> thanks
> -- PMM
>

Thanks Peter,

I'll send a patch shortly.

Cheers,
Edgar


Re: [PULL 00/36] Migration 20220128 patches

2022-01-30 Thread Peter Maydell
On Fri, 28 Jan 2022 at 18:31, Juan Quintela  wrote:
>
> The following changes since commit b367db48126d4ee14579af6cf5cdbffeb9496627:
>
>   Merge remote-tracking branch 'remotes/aperard/tags/pull-xen-20220127' into 
> staging (2022-01-28 11:05:29 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/juan.quintela/qemu.git 
> tags/migration-20220128-pull-request
>
> for you to fetch changes up to 476ebf77fe8909ded10046edf26685bc28438162:
>
>   migration: Move temp page setup and cleanup into separate functions 
> (2022-01-28 15:38:23 +0100)
>
> 
> Migration Pull request (Take 2)
>
> Hi
>
> This time I have disabled vmstate canary patches form Dave Gilbert.
>
> Let's see if it works.
>
> Later, Juan.
>



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM



[PATCH v4] hw/sensor: Add lsm303dlhc magnetometer device

2022-01-30 Thread Kevin Townsend
This commit adds emulation of the magnetometer on the LSM303DLHC.
It allows the magnetometer's X, Y and Z outputs to be set via the
mag-x, mag-y and mag-z properties, as well as the 12-bit
temperature output via the temperature property. Sensor can be
enabled with 'CONFIG_LSM303DLHC_MAG=y'.

Signed-off-by: Kevin Townsend 
---
 hw/sensor/Kconfig |   4 +
 hw/sensor/lsm303dlhc_mag.c| 556 ++
 hw/sensor/meson.build |   1 +
 tests/qtest/lsm303dlhc-mag-test.c | 148 
 tests/qtest/meson.build   |   1 +
 5 files changed, 710 insertions(+)
 create mode 100644 hw/sensor/lsm303dlhc_mag.c
 create mode 100644 tests/qtest/lsm303dlhc-mag-test.c

diff --git a/hw/sensor/Kconfig b/hw/sensor/Kconfig
index 9c8a049b06..b317f91b7b 100644
--- a/hw/sensor/Kconfig
+++ b/hw/sensor/Kconfig
@@ -21,3 +21,7 @@ config ADM1272
 config MAX34451
 bool
 depends on I2C
+
+config LSM303DLHC_MAG
+bool
+depends on I2C
diff --git a/hw/sensor/lsm303dlhc_mag.c b/hw/sensor/lsm303dlhc_mag.c
new file mode 100644
index 00..4c98ddbf20
--- /dev/null
+++ b/hw/sensor/lsm303dlhc_mag.c
@@ -0,0 +1,556 @@
+/*
+ * LSM303DLHC I2C magnetometer.
+ *
+ * Copyright (C) 2021 Linaro Ltd.
+ * Written by Kevin Townsend 
+ *
+ * Based on: https://www.st.com/resource/en/datasheet/lsm303dlhc.pdf
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+/*
+ * The I2C address associated with this device is set on the command-line when
+ * initialising the machine, but the following address is standard: 0x1E.
+ *
+ * Get and set functions for 'mag-x', 'mag-y' and 'mag-z' assume that
+ * 1 = 0.001 uT. (NOTE the 1 gauss = 100 uT, so setting a value of 100,000
+ * would be equal to 1 gauss or 100 uT.)
+ *
+ * Get and set functions for 'temperature' assume that 1 = 0.001 C, so 23.6 C
+ * would be equal to 23600.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/i2c/i2c.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "qemu/module.h"
+#include "qemu/log.h"
+#include "qemu/bswap.h"
+
+enum LSM303DLHCMagReg {
+LSM303DLHC_MAG_REG_CRA  = 0x00,
+LSM303DLHC_MAG_REG_CRB  = 0x01,
+LSM303DLHC_MAG_REG_MR   = 0x02,
+LSM303DLHC_MAG_REG_OUT_X_H  = 0x03,
+LSM303DLHC_MAG_REG_OUT_X_L  = 0x04,
+LSM303DLHC_MAG_REG_OUT_Z_H  = 0x05,
+LSM303DLHC_MAG_REG_OUT_Z_L  = 0x06,
+LSM303DLHC_MAG_REG_OUT_Y_H  = 0x07,
+LSM303DLHC_MAG_REG_OUT_Y_L  = 0x08,
+LSM303DLHC_MAG_REG_SR   = 0x09,
+LSM303DLHC_MAG_REG_IRA  = 0x0A,
+LSM303DLHC_MAG_REG_IRB  = 0x0B,
+LSM303DLHC_MAG_REG_IRC  = 0x0C,
+LSM303DLHC_MAG_REG_TEMP_OUT_H   = 0x31,
+LSM303DLHC_MAG_REG_TEMP_OUT_L   = 0x32
+};
+
+typedef struct LSM303DLHCMagState {
+I2CSlave parent_obj;
+uint8_t cra;
+uint8_t crb;
+uint8_t mr;
+int16_t x;
+int16_t z;
+int16_t y;
+int16_t x_lock;
+int16_t z_lock;
+int16_t y_lock;
+uint8_t sr;
+uint8_t ira;
+uint8_t irb;
+uint8_t irc;
+int16_t temperature;
+int16_t temperature_lock;
+uint8_t len;
+uint8_t buf;
+uint8_t pointer;
+} LSM303DLHCMagState;
+
+#define TYPE_LSM303DLHC_MAG "lsm303dlhc_mag"
+OBJECT_DECLARE_SIMPLE_TYPE(LSM303DLHCMagState, LSM303DLHC_MAG)
+
+/*
+ * Conversion factor from Gauss to sensor values for each GN gain setting,
+ * in units "lsb per Gauss" (see data sheet table 3). There is no documented
+ * behaviour if the GN setting in CRB is incorrectly set to 0b000;
+ * we arbitrarily make it the same as 0b001.
+ */
+uint32_t xy_gain[] = { 1100, 1100, 855, 670, 450, 400, 330, 230 };
+uint32_t z_gain[] = { 980, 980, 760, 600, 400, 355, 295, 205 };
+
+static void lsm303dlhc_mag_get_x(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+LSM303DLHCMagState *s = LSM303DLHC_MAG(obj);
+int gm = extract32(s->crb, 5, 3);
+
+/* Convert to uT where 1000 = 1 uT. Conversion factor depends on gain. */
+int64_t value = muldiv64(s->x, 10, xy_gain[gm]);
+visit_type_int(v, name, , errp);
+}
+
+static void lsm303dlhc_mag_get_y(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+LSM303DLHCMagState *s = LSM303DLHC_MAG(obj);
+int gm = extract32(s->crb, 5, 3);
+
+/* Convert to uT where 1000 = 1 uT. Conversion factor depends on gain. */
+int64_t value = muldiv64(s->y, 10, xy_gain[gm]);
+visit_type_int(v, name, , errp);
+}
+
+static void lsm303dlhc_mag_get_z(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+LSM303DLHCMagState *s = LSM303DLHC_MAG(obj);
+int gm = extract32(s->crb, 5, 3);
+
+/* Convert to uT where 1000 = 1 uT. Conversion factor depends on gain. */
+int64_t value = muldiv64(s->z, 10, z_gain[gm]);
+visit_type_int(v, name, , errp);
+}
+