Re: [PATCH-for-9.0 25/25] hw/pci-host/raven: Propagate error in raven_realize()

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

When an Error** reference is available, it is better to
propagate local errors, rather then using generic ones,
which might terminate the whole QEMU process.

Signed-off-by: Philippe Mathieu-Daudé 
---
hw/pci-host/raven.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/raven.c b/hw/pci-host/raven.c
index 86c3a49087..e34ce47874 100644
--- a/hw/pci-host/raven.c
+++ b/hw/pci-host/raven.c
@@ -345,8 +345,10 @@ static void raven_realize(PCIDevice *d, Error **errp)
d->config[PCI_LATENCY_TIMER] = 0x10;
d->config[PCI_CAPABILITY_LIST] = 0x00;

-memory_region_init_rom_nomigrate(>bios, OBJECT(s), "bios", BIOS_SIZE,
- _fatal);
+if (!memory_region_init_rom_nomigrate(>bios, OBJECT(s), "bios",
+  BIOS_SIZE, errp)) {
+return;
+}
memory_region_add_subregion(get_system_memory(), (uint32_t)(-BIOS_SIZE),
>bios);
if (s->bios_name) {
--
2.41.0


Reviewed-by: Manos Pitsidianakis 

[PATCH] ppc/spapr: Initialize max_cpus limit to an allowed usable limit.

[Harsh Prateek Bora]

Initialize the machine specific max_cpus limit to a usable limit 4096.
Keeping between 4096 to 8192 will throw IRQ not free error due to XIVE
limitation and keeping beyond 8192 will hit assert in tcg_region_init
or spapr_xive_claim_irq.

Logs:

Without patch fix:

[root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=4097
qemu-system-ppc64: IRQ 4096 is not free
[root@host build]#

On LPAR:
[root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=8193
**
ERROR:../tcg/region.c:774:tcg_region_init: assertion failed:
(region_size >= 2 * page_size)
Bail out! ERROR:../tcg/region.c:774:tcg_region_init: assertion failed:
(region_size >= 2 * page_size)
Aborted (core dumped)
[root@host build]#

On x86:
[root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=8193
qemu-system-ppc64: ../hw/intc/spapr_xive.c:596: spapr_xive_claim_irq:
Assertion `lisn < xive->nr_irqs' failed.
Aborted (core dumped)
[root@host build]#

With patch fix:
[root@host build]# qemu-system-ppc64 -accel tcg -smp 10,maxcpus=4097
qemu-system-ppc64: Invalid SMP CPUs 4097. The max CPUs supported by
machine 'pseries-8.2' is 4096
[root@host build]#

Signed-off-by: Harsh Prateek Bora 
---
 hw/ppc/spapr.c | 9 +++--
 include/hw/ppc/spapr.h | 1 +
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index df09aa9d6a..1995949ea5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4647,13 +4647,10 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 mc->block_default_type = IF_SCSI;
 
 /*
- * Setting max_cpus to INT32_MAX. Both KVM and TCG max_cpus values
- * should be limited by the host capability instead of hardcoded.
- * max_cpus for KVM guests will be checked in kvm_init(), and TCG
- * guests are welcome to have as many CPUs as the host are capable
- * of emulate.
+ * While KVM determines max cpus in kvm_init() using kvm_max_vcpus(),
+ * In TCG the limit is restricted by max-irqs setup by XIVE which is 4096.
  */
-mc->max_cpus = INT32_MAX;
+mc->max_cpus = SPAPR_MAX_CPUS;
 
 mc->no_parallel = 1;
 mc->default_boot_order = "";
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index e91791a1a9..210849a494 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -23,6 +23,7 @@ typedef struct SpaprPendingHpt SpaprPendingHpt;
 
 typedef struct Vof Vof;
 
+#define SPAPR_MAX_CPUS  4096
 #define HPTE64_V_HPTE_DIRTY 0x0040ULL
 #define SPAPR_ENTRY_POINT   0x100
 
-- 
2.39.3

Re: [PATCH-for-9.0 24/25] hw/nvram: Simplify memory_region_init_rom_device() calls

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

Mechanical change using the following coccinelle script:

@@
expression mr, owner, arg3, arg4, arg5, arg6, errp;
@@
-   memory_region_init_rom_device(mr, owner, arg3, arg4, arg5, arg6, );
   if (
-   errp
+   !memory_region_init_rom_device(mr, owner, arg3, arg4, arg5, arg6, )
   ) {
   ...
   return;
   }

and removing the local Error variable.

Signed-off-by: Philippe Mathieu-Daudé 
---
hw/nvram/nrf51_nvm.c | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/nvram/nrf51_nvm.c b/hw/nvram/nrf51_nvm.c
index 7f1db8c423..7b25becd49 100644
--- a/hw/nvram/nrf51_nvm.c
+++ b/hw/nvram/nrf51_nvm.c
@@ -336,12 +336,9 @@ static void nrf51_nvm_init(Object *obj)
static void nrf51_nvm_realize(DeviceState *dev, Error **errp)
{
NRF51NVMState *s = NRF51_NVM(dev);
-Error *err = NULL;

-memory_region_init_rom_device(>flash, OBJECT(dev), _ops, s,
-"nrf51_soc.flash", s->flash_size, );
-if (err) {
-error_propagate(errp, err);
+if (!memory_region_init_rom_device(>flash, OBJECT(dev), _ops, s,
+   "nrf51_soc.flash", s->flash_size, 
errp)) {
return;
}

--


Reviewed-by: Manos Pitsidianakis 

Re: [PATCH-for-9.0 23/25] hw/misc: Simplify memory_region_init_ram_from_fd() calls

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

Mechanical change using the following coccinelle script:

@@
expression mr, owner, arg3, arg4, arg5, arg6, arg7, errp;
@@
-   memory_region_init_ram_from_fd(mr, owner, arg3, arg4, arg5, arg6, arg7, 
);
   if (
-   errp
+   !memory_region_init_ram_from_fd(mr, owner, arg3, arg4, arg5, arg6, arg7, 
)
   ) {
   ...
   return;
   }

and removing the local Error variable.

Signed-off-by: Philippe Mathieu-Daudé 
---



Reviewed-by: Manos Pitsidianakis 

Re: [PATCH-for-9.0 21/25] hw/arm: Simplify memory_region_init_rom() calls

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

Mechanical change using the following coccinelle script:

@@
expression mr, owner, arg3, arg4, errp;
@@
-   memory_region_init_rom(mr, owner, arg3, arg4, );
   if (
-   errp
+   !memory_region_init_rom(mr, owner, arg3, arg4, )
   ) {
   ...
   return;
   }

and removing the local Error variable.

Signed-off-by: Philippe Mathieu-Daudé 
---



Reviewed-by: Manos Pitsidianakis 

Re: [PATCH v4 05/11] ppc/pnv: Wire up pca9552 GPIO pins for PCIe hotplug power control

[Cédric Le Goater]

Adding Reza.

On 11/21/23 21:03, Miles Glenn wrote:

On Tue, 2023-11-21 at 19:36 +0100, Cédric Le Goater wrote:

On 11/21/23 00:51, Glenn Miles wrote:

For power10-rainier, a pca9552 device is used for PCIe slot hotplug
power control by the Power Hypervisor code.  The code expects that
some time after it enables power to a PCIe slot by asserting one of
the pca9552 GPIO pins 0-4, it should see a "power good" signal
asserted
on one of pca9552 GPIO pins 5-9.


And this is what OPAL is not doing :


https://github.com/open-power/skiboot/blob/master/platforms/astbmc/rainier.c#L65


Correct ?



Ah, yes, I believe you are correct!

Thanks,

C.

Re: [PATCH-for-9.0 20/25] hw: Simplify memory_region_init_ram() calls

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

Mechanical change using the following coccinelle script:

@@
expression mr, owner, arg3, arg4, errp;
@@
-   memory_region_init_ram(mr, owner, arg3, arg4, );
   if (
-   errp
+   !memory_region_init_ram(mr, owner, arg3, arg4, )
   ) {
   ...
   return;
   }

and removing the local Error variable.

Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Manos Pitsidianakis 

Re: [PATCH-for-9.0 19/25] misc: Simplify qemu_prealloc_mem() calls

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

Signed-off-by: Philippe Mathieu-Daudé 
---


Commit message missing but indeed there's not much to say

Reviewed-by: Manos Pitsidianakis 

Re: [PATCH-for-9.0 17/25] backends: Reduce variable scope in host_memory_backend_memory_complete

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

Reduce the _err variable use and remove the 'out:' label.

Signed-off-by: Philippe Mathieu-Daudé 
---
backends/hostmem.c | 8 
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/backends/hostmem.c b/backends/hostmem.c
index 3f8eb936d7..1b0043a0d9 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -324,7 +324,6 @@ host_memory_backend_memory_complete(UserCreatable *uc, 
Error **errp)
{
HostMemoryBackend *backend = MEMORY_BACKEND(uc);
HostMemoryBackendClass *bc = MEMORY_BACKEND_GET_CLASS(uc);
-Error *local_err = NULL;
void *ptr;
uint64_t sz;

@@ -400,15 +399,16 @@ host_memory_backend_memory_complete(UserCreatable *uc, 
Error **errp)
 * specified NUMA policy in place.
 */
if (backend->prealloc) {
+Error *local_err = NULL;
+
qemu_prealloc_mem(memory_region_get_fd(>mr), ptr, sz,
  backend->prealloc_threads,
  backend->prealloc_context, _err);
if (local_err) {
-goto out;
+error_propagate(errp, local_err);
+return;
}
}
-out:
-error_propagate(errp, local_err);
}

static bool
--


Reviewed-by: Manos Pitsidianakis 

[RFC PATCH v7] ppc: Enable 2nd DAWR support on p10

[Shivaprasad G Bhat]

Extend the existing watchpoint facility from TCG DAWR0 emulation
to DAWR1 on POWER10.

As per the PAPR, bit 0 of byte 64 in pa-features property
indicates availability of 2nd DAWR registers. i.e. If this bit is set, 2nd
DAWR is present, otherwise not. Use KVM_CAP_PPC_DAWR1 capability to find
whether kvm supports 2nd DAWR or not. If it's supported, allow user to set
the pa-feature bit in guest DT using cap-dawr1 machine capability.

Signed-off-by: Ravi Bangoria 
Signed-off-by: Shivaprasad G Bhat 
---
Changelog:
v6: 
https://lore.kernel.org/qemu-devel/168871963321.58984.15628382614621248470.stgit@ltcd89-lp2/
v6->v7:
  - Sorry about the delay in sending out this version, I have dropped the
Reviewed-bys as suggested and converted the patch to RFC back again.
  - Added the TCG support. Basically, converted the existing DAWR0 support
routines into macros for reuse by the DAWR1. Let me know if the macro
conversions should be moved to a separate independent patch.
  - As the dawr1 works on TCG, the checks in cap_dawr1_apply() report a warning
now only for P9 or P9 compat modes for both KVM and TCG use cases.
  - 'make test' passes for caps checks. Also, as suggested by Greg Kurz, the
'make test' after making the DAWR1 default 'on' and updating defaut cpu
to Power10, shows no failures.

v5: 
https://lore.kernel.org/all/20210412114433.129702-1-ravi.bango...@linux.ibm.com/
v5->v6:
  - The other patches in the original series already merged.
  - Rebased to the top of the tree. So, the gen_spr_book3s_310_dbg() is renamed
to register_book3s_310_dbg_sprs() and moved to cpu_init.c accordingly.
  - No functional changes.

v4: 
https://lore.kernel.org/r/20210406053833.282907-1-ravi.bango...@linux.ibm.com
v3->v4:
  - Make error message more proper.

v3: https://lore.kernel.org/r/20210330095350.36309-1-ravi.bango...@linux.ibm.com
v3->v4:
  - spapr_dt_pa_features(): POWER10 processor is compatible with 3.0
(PCR_COMPAT_3_00). No need to ppc_check_compat(3_10) for now as
ppc_check_compati(3_00) will also be true. ppc_check_compat(3_10)
can be added while introducing pa_features_310 in future.
  - Use error_append_hint() for hints. Also add ERRP_GUARD().
  - Add kvmppc_set_cap_dawr1() stub function for CONFIG_KVM=n.

v2: 
https://lore.kernel.org/r/20210329041906.213991-1-ravi.bango...@linux.ibm.com
v2->v3:
  - Don't introduce pa_features_310[], instead, reuse pa_features_300[]
for 3.1 guests, as there is no difference between initial values of
them atm.
  - Call gen_spr_book3s_310_dbg() from init_proc_POWER10() instead of
init_proc_POWER8(). Also, Don't call gen_spr_book3s_207_dbg() from
gen_spr_book3s_310_dbg() as init_proc_POWER10() already calls it.

v1: 
https://lore.kernel.org/r/20200723104220.314671-1-ravi.bango...@linux.ibm.com
v1->v2:
  - Introduce machine capability cap-dawr1 to enable/disable
the feature. By default, 2nd DAWR is OFF for guests even
when host kvm supports it. User has to manually enable it
with -machine cap-dawr1=on if he wishes to use it.
  - Split the header file changes into separate patch. (Sync
headers from v5.12-rc3)

 hw/ppc/spapr.c   |7 ++-
 hw/ppc/spapr_caps.c  |   35 ++
 hw/ppc/spapr_hcall.c |   50 
 include/hw/ppc/spapr.h   |6 ++
 target/ppc/cpu.c |  114 +-
 target/ppc/cpu.h |6 ++
 target/ppc/cpu_init.c|   15 ++
 target/ppc/excp_helper.c |   61 ++---
 target/ppc/helper.h  |2 +
 target/ppc/kvm.c |   12 +
 target/ppc/kvm_ppc.h |   12 +
 target/ppc/machine.c |1
 target/ppc/misc_helper.c |   20 ++--
 target/ppc/spr_common.h  |2 +
 target/ppc/translate.c   |   25 +++---
 15 files changed, 253 insertions(+), 115 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index df09aa9d6a..c1cb47464b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -262,7 +262,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
 0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
 /* 54: DecFP, 56: DecI, 58: SHA */
 0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
-/* 60: NM atomic, 62: RNG */
+/* 60: NM atomic, 62: RNG, 64: DAWR1 (ISA 3.1) */
 0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
 };
 uint8_t *pa_features = NULL;
@@ -303,6 +303,9 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
  * in pa-features. So hide it from them. */
 pa_features[40 + 2] &= ~0x80; /* Radix MMU */
 }
+if (spapr_get_cap(spapr, SPAPR_CAP_DAWR1)) {
+pa_features[66] |= 0x80;
+}

 _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
 }
@@ -2138,6 +2141,7 @@ static const VMStateDescription vmstate_spapr = {
 _spapr_cap_fwnmi,
 _spapr_fwnmi,
 _spapr_cap_rpt_invalidate,
+_spapr_cap_dawr1,
 NULL
  

[PATCH 2/3] tests/unit/test-qmp-event: Simplify event emission check

[Markus Armbruster]

The generated qapi_event_send_FOO() call an event emitter function.
It's test_qapi_event_emit() in this test.  It compares the actual
event to the expected event, and sets a flag to record it was called.
The test functions set expected data and clear the flag before calling
qapi_event_send_FOO(), and check the flag afterwards.

Make test_qapi_event_emit() consume expected data, and the test
functions check it was consumed.  Delete the flag.  This is simpler.
It also catches extraneous calls of test_qapi_event_emit().  Catching
that is not worthwhile, but since the cost is negative...

Signed-off-by: Markus Armbruster 
---
 tests/unit/test-qmp-event.c | 30 ++
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c
index c2c44687d5..5c9837e849 100644
--- a/tests/unit/test-qmp-event.c
+++ b/tests/unit/test-qmp-event.c
@@ -26,7 +26,6 @@
 
 typedef struct TestEventData {
 QDict *expect;
-bool emitted;
 } TestEventData;
 
 TestEventData *test_event_data;
@@ -36,6 +35,8 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d)
 QDict *t;
 int64_t s, ms;
 
+g_assert(test_event_data->expect);
+
 /* Verify that we have timestamp, then remove it to compare other fields */
 t = qdict_get_qdict(d, "timestamp");
 g_assert(t);
@@ -52,7 +53,8 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d)
 qdict_del(d, "timestamp");
 
 g_assert(qobject_is_equal(QOBJECT(d), QOBJECT(test_event_data->expect)));
-test_event_data->emitted = true;
+qobject_unref(test_event_data->expect);
+test_event_data->expect = NULL;
 }
 
 static void event_prepare(TestEventData *data,
@@ -83,8 +85,7 @@ static void test_event_a(TestEventData *data,
 {
 data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_A' }");
 qapi_event_send_event_a();
-g_assert(data->emitted);
-qobject_unref(data->expect);
+g_assert(!data->expect);
 }
 
 static void test_event_b(TestEventData *data,
@@ -92,8 +93,7 @@ static void test_event_b(TestEventData *data,
 {
 data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_B' }");
 qapi_event_send_event_b();
-g_assert(data->emitted);
-qobject_unref(data->expect);
+g_assert(!data->expect);
 }
 
 static void test_event_c(TestEventData *data,
@@ -105,8 +105,7 @@ static void test_event_c(TestEventData *data,
 "{ 'event': 'EVENT_C', 'data': {"
 " 'a': 1, 'b': { 'integer': 2, 'string': 'test1' }, 'c': 'test2' } }");
 qapi_event_send_event_c(true, 1, , "test2");
-g_assert(data->emitted);
-qobject_unref(data->expect);
+g_assert(!data->expect);
 }
 
 /* Complex type */
@@ -131,8 +130,7 @@ static void test_event_d(TestEventData *data,
 "  'string': 'test2', 'enum2': 'value2' },"
 " 'b': 'test3', 'enum3': 'value3' } }");
 qapi_event_send_event_d(, "test3", NULL, true, ENUM_ONE_VALUE3);
-g_assert(data->emitted);
-qobject_unref(data->expect);
+g_assert(!data->expect);
 }
 
 static void test_event_deprecated(TestEventData *data, const void *unused)
@@ -142,15 +140,11 @@ static void test_event_deprecated(TestEventData *data, 
const void *unused)
 memset(_policy, 0, sizeof(compat_policy));
 
 qapi_event_send_test_event_features1();
-g_assert(data->emitted);
+g_assert(!data->expect);
 
 compat_policy.has_deprecated_output = true;
 compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
-data->emitted = false;
 qapi_event_send_test_event_features1();
-g_assert(!data->emitted);
-
-qobject_unref(data->expect);
 }
 
 static void test_event_deprecated_data(TestEventData *data, const void *unused)
@@ -160,17 +154,13 @@ static void test_event_deprecated_data(TestEventData 
*data, const void *unused)
 data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0',"
" 'data': { 'foo': 42 } }");
 qapi_event_send_test_event_features0(42);
-g_assert(data->emitted);
+g_assert(!data->expect);
 
-qobject_unref(data->expect);
 
 compat_policy.has_deprecated_output = true;
 compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
 data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0' 
}");
 qapi_event_send_test_event_features0(42);
-g_assert(data->emitted);
-
-qobject_unref(data->expect);
 }
 
 int main(int argc, char **argv)
-- 
2.41.0

[PATCH 1/3] tests/unit/test-qmp-event: Drop superfluous mutex

[Markus Armbruster]

Mutex @test_event_lock is held from fixture setup to teardown,
protecting global variable @test_event_data.  But tests always run one
after the other, so this is superfluous.  It also confuses Coverity.
Drop the mutex.

Fixes: CID 1527425
Signed-off-by: Markus Armbruster 
---
 tests/unit/test-qmp-event.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c
index 3626d2372f..c2c44687d5 100644
--- a/tests/unit/test-qmp-event.c
+++ b/tests/unit/test-qmp-event.c
@@ -30,7 +30,6 @@ typedef struct TestEventData {
 } TestEventData;
 
 TestEventData *test_event_data;
-static GMutex test_event_lock;
 
 void test_qapi_event_emit(test_QAPIEvent event, QDict *d)
 {
@@ -59,9 +58,6 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d)
 static void event_prepare(TestEventData *data,
   const void *unused)
 {
-/* Global variable test_event_data was used to pass the expectation, so
-   test cases can't be executed at same time. */
-g_mutex_lock(_event_lock);
 test_event_data = data;
 }
 
@@ -69,7 +65,6 @@ static void event_teardown(TestEventData *data,
const void *unused)
 {
 test_event_data = NULL;
-g_mutex_unlock(_event_lock);
 }
 
 static void event_test_add(const char *testpath,
-- 
2.41.0

[PATCH 3/3] tests/unit/test-qmp-event: Replace fixture by global variables

[Markus Armbruster]

The fixture buys us exactly nothing, as we need a global variable
anyway, for test_qapi_event_emit().  Drop it.

Signed-off-by: Markus Armbruster 
---
 tests/unit/test-qmp-event.c | 91 -
 1 file changed, 30 insertions(+), 61 deletions(-)

diff --git a/tests/unit/test-qmp-event.c b/tests/unit/test-qmp-event.c
index 5c9837e849..08e95a382b 100644
--- a/tests/unit/test-qmp-event.c
+++ b/tests/unit/test-qmp-event.c
@@ -24,18 +24,14 @@
 #include "test-qapi-events.h"
 #include "test-qapi-emit-events.h"
 
-typedef struct TestEventData {
-QDict *expect;
-} TestEventData;
-
-TestEventData *test_event_data;
+static QDict *expected_event;
 
 void test_qapi_event_emit(test_QAPIEvent event, QDict *d)
 {
 QDict *t;
 int64_t s, ms;
 
-g_assert(test_event_data->expect);
+g_assert(expected_event);
 
 /* Verify that we have timestamp, then remove it to compare other fields */
 t = qdict_get_qdict(d, "timestamp");
@@ -52,65 +48,38 @@ void test_qapi_event_emit(test_QAPIEvent event, QDict *d)
 
 qdict_del(d, "timestamp");
 
-g_assert(qobject_is_equal(QOBJECT(d), QOBJECT(test_event_data->expect)));
-qobject_unref(test_event_data->expect);
-test_event_data->expect = NULL;
+g_assert(qobject_is_equal(QOBJECT(d), QOBJECT(expected_event)));
+qobject_unref(expected_event);
+expected_event = NULL;
 }
 
-static void event_prepare(TestEventData *data,
-  const void *unused)
+static void test_event_a(void)
 {
-test_event_data = data;
-}
-
-static void event_teardown(TestEventData *data,
-   const void *unused)
-{
-test_event_data = NULL;
-}
-
-static void event_test_add(const char *testpath,
-   void (*test_func)(TestEventData *data,
- const void *user_data))
-{
-g_test_add(testpath, TestEventData, NULL, event_prepare, test_func,
-   event_teardown);
-}
-
-
-/* Test cases */
-
-static void test_event_a(TestEventData *data,
- const void *unused)
-{
-data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_A' }");
+expected_event = qdict_from_jsonf_nofail("{ 'event': 'EVENT_A' }");
 qapi_event_send_event_a();
-g_assert(!data->expect);
+g_assert(!expected_event);
 }
 
-static void test_event_b(TestEventData *data,
- const void *unused)
+static void test_event_b(void)
 {
-data->expect = qdict_from_jsonf_nofail("{ 'event': 'EVENT_B' }");
+expected_event = qdict_from_jsonf_nofail("{ 'event': 'EVENT_B' }");
 qapi_event_send_event_b();
-g_assert(!data->expect);
+g_assert(!expected_event);
 }
 
-static void test_event_c(TestEventData *data,
- const void *unused)
+static void test_event_c(void)
 {
 UserDefOne b = { .integer = 2, .string = (char *)"test1" };
 
-data->expect = qdict_from_jsonf_nofail(
+expected_event = qdict_from_jsonf_nofail(
 "{ 'event': 'EVENT_C', 'data': {"
 " 'a': 1, 'b': { 'integer': 2, 'string': 'test1' }, 'c': 'test2' } }");
 qapi_event_send_event_c(true, 1, , "test2");
-g_assert(!data->expect);
+g_assert(!expected_event);
 }
 
 /* Complex type */
-static void test_event_d(TestEventData *data,
- const void *unused)
+static void test_event_d(void)
 {
 UserDefOne struct1 = {
 .integer = 2, .string = (char *)"test1",
@@ -123,43 +92,43 @@ static void test_event_d(TestEventData *data,
 .enum2 = ENUM_ONE_VALUE2,
 };
 
-data->expect = qdict_from_jsonf_nofail(
+expected_event = qdict_from_jsonf_nofail(
 "{ 'event': 'EVENT_D', 'data': {"
 " 'a': {"
 "  'struct1': { 'integer': 2, 'string': 'test1', 'enum1': 'value1' },"
 "  'string': 'test2', 'enum2': 'value2' },"
 " 'b': 'test3', 'enum3': 'value3' } }");
 qapi_event_send_event_d(, "test3", NULL, true, ENUM_ONE_VALUE3);
-g_assert(!data->expect);
+g_assert(!expected_event);
 }
 
-static void test_event_deprecated(TestEventData *data, const void *unused)
+static void test_event_deprecated(void)
 {
-data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES1' 
}");
+expected_event = qdict_from_jsonf_nofail("{ 'event': 
'TEST_EVENT_FEATURES1' }");
 
 memset(_policy, 0, sizeof(compat_policy));
 
 qapi_event_send_test_event_features1();
-g_assert(!data->expect);
+g_assert(!expected_event);
 
 compat_policy.has_deprecated_output = true;
 compat_policy.deprecated_output = COMPAT_POLICY_OUTPUT_HIDE;
 qapi_event_send_test_event_features1();
 }
 
-static void test_event_deprecated_data(TestEventData *data, const void *unused)
+static void test_event_deprecated_data(void)
 {
 memset(_policy, 0, sizeof(compat_policy));
 
-data->expect = qdict_from_jsonf_nofail("{ 'event': 'TEST_EVENT_FEATURES0',"
+expected_event = qdict_from_jsonf_nofail("{ 'event': 

[PATCH 0/3] tests/unit/test-qmp-event: Unconfuse Coverity & simplify

[Markus Armbruster]

Markus Armbruster (3):
  tests/unit/test-qmp-event: Drop superfluous mutex
  tests/unit/test-qmp-event: Simplify event emission check
  tests/unit/test-qmp-event: Replace fixture by global variables

 tests/unit/test-qmp-event.c | 108 +++-
 1 file changed, 31 insertions(+), 77 deletions(-)

-- 
2.41.0

Re: [PATCH-for-9.0 16/25] backends: Have HostMemoryBackendClass::alloc() handler return a boolean

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have cpu_exec_realizefn()
return a boolean indicating whether an error is set or not.

Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Manos Pitsidianakis 

Re: [PATCH-for-9.0 15/25] backends: Simplify host_memory_backend_memory_complete()

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

Return early if bc->alloc is NULL. De-indent the if() ladder.

Note, this avoids a pointless call to error_propagate() with
errp=NULL at the 'out:' label.

Change trivial when reviewed with 'git-diff --ignore-all-space'.

Signed-off-by: Philippe Mathieu-Daudé 
---


Reviewed-by: Manos Pitsidianakis 

Re: [PATCH-for-9.0 14/25] backends: Use g_autofree in HostMemoryBackendClass::alloc() handlers

[Manos Pitsidianakis]

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé  wrote:

In preparation of having HostMemoryBackendClass::alloc() handlers
return a boolean, have them use g_autofree.

Signed-off-by: Philippe Mathieu-Daudé 
---



Reviewed-by: Manos Pitsidianakis 

Re: [PATCH qemu v2] Fixing the basic functionality of STM32 timers

[Alistair Francis]

On Tue, Oct 31, 2023 at 6:35 AM ~lbryndza  wrote:
>
> From: Lucjan Bryndza 
>
> The current implementation of timers does not work properly
> even in basic functionality. A counter configured to report
> an interrupt every 10ms reports the first interrupts after a
> few seconds.  There are also no properly implemented count up and
> count down modes. This commit fixes bugs with interrupt
> reporting and implements the basic modes of the counter's
> time-base block.
>
> Signed-off-by: Lucjan Bryndza 

Thanks. This is already easier to follow than the first version. It
still helps to split this up, it's relatively long and difficult to
follow the changes.

You have done a great job of explaining what the current problem is in
the commit message, but do you mind explaining a bit more details
about your fix?

I know it might seem like a pain, but I am pretty much reviewing the
STM32 work in my spare time and I haven't worked on it directly in
awhile. The simpler you can make the changes the easier and quicker it
is to review. As you have noticed, timers are tricky to get right, so
we want to try and make sure it's correct this time :)

> ---
>  hw/arm/stm32f405_soc.c |   2 +-
>  hw/timer/stm32f2xx_timer.c | 262 +++--
>  include/hw/timer/stm32f2xx_timer.h |  23 ++-
>  3 files changed, 189 insertions(+), 98 deletions(-)
>
> diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
> index cef23d7ee4..69316181b3 100644
> --- a/hw/arm/stm32f405_soc.c
> +++ b/hw/arm/stm32f405_soc.c
> @@ -183,7 +183,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, 
> Error **errp)
>  /* Timer 2 to 5 */
>  for (i = 0; i < STM_NUM_TIMERS; i++) {
>  dev = DEVICE(&(s->timer[i]));
> -qdev_prop_set_uint64(dev, "clock-frequency", 10);
> +qdev_prop_set_uint64(dev, "clock-frequency", 4800);
>  if (!sysbus_realize(SYS_BUS_DEVICE(>timer[i]), errp)) {
>  return;
>  }
> diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
> index ba8694dcd3..9a992231fa 100644
> --- a/hw/timer/stm32f2xx_timer.c
> +++ b/hw/timer/stm32f2xx_timer.c
> @@ -23,12 +23,17 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qapi/error.h"
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/timer/stm32f2xx_timer.h"
>  #include "migration/vmstate.h"
>  #include "qemu/log.h"
>  #include "qemu/module.h"
> +#include "qemu/typedefs.h"
> +#include "qemu/timer.h"
> +#include "qemu/main-loop.h"
> +#include "sysemu/dma.h"
>
>  #ifndef STM_TIMER_ERR_DEBUG
>  #define STM_TIMER_ERR_DEBUG 0
> @@ -42,63 +47,87 @@
>
>  #define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
>
> -static void stm32f2xx_timer_set_alarm(STM32F2XXTimerState *s, int64_t now);
>
> -static void stm32f2xx_timer_interrupt(void *opaque)
> +static uint32_t stm32f2xx_timer_get_count(STM32F2XXTimerState *s)
>  {
> -STM32F2XXTimerState *s = opaque;
> -
> -DB_PRINT("Interrupt\n");
> -
> -if (s->tim_dier & TIM_DIER_UIE && s->tim_cr1 & TIM_CR1_CEN) {
> -s->tim_sr |= 1;
> -qemu_irq_pulse(s->irq);
> -stm32f2xx_timer_set_alarm(s, s->hit_time);
> -}
> -
> -if (s->tim_ccmr1 & (TIM_CCMR1_OC2M2 | TIM_CCMR1_OC2M1) &&
> -!(s->tim_ccmr1 & TIM_CCMR1_OC2M0) &&
> -s->tim_ccmr1 & TIM_CCMR1_OC2PE &&
> -s->tim_ccer & TIM_CCER_CC2E) {
> -/* PWM 2 - Mode 1 */
> -DB_PRINT("PWM2 Duty Cycle: %d%%\n",
> -s->tim_ccr2 / (100 * (s->tim_psc + 1)));
> +uint64_t cnt = ptimer_get_count(s->timer);
> +if (s->count_mode == TIMER_UP_COUNT) {
> +return s->tim_arr - (cnt & 0x);
> +} else {
> +return cnt & 0x;
>  }
>  }
>
> -static inline int64_t stm32f2xx_ns_to_ticks(STM32F2XXTimerState *s, int64_t 
> t)
> +
> +static void stm32f2xx_timer_set_count(STM32F2XXTimerState *s, uint32_t cnt)
>  {
> -return muldiv64(t, s->freq_hz, 10ULL) / (s->tim_psc + 1);
> +if (s->count_mode == TIMER_UP_COUNT) {
> +ptimer_set_count(s->timer, s->tim_arr - (cnt & 0x));
> +} else {
> +ptimer_set_count(s->timer, cnt & 0x);
> +}
>  }
>
> -static void stm32f2xx_timer_set_alarm(STM32F2XXTimerState *s, int64_t now)
> +static void stm32f2xx_timer_update(STM32F2XXTimerState *s)
>  {
> -uint64_t ticks;
> -int64_t now_ticks;
> +if (s->tim_cr1 & TIM_CR1_DIR) {
> +s->count_mode = TIMER_DOWN_COUNT;
> +} else {
> +s->count_mode = TIMER_UP_COUNT;
> +}
>
> -if (s->tim_arr == 0) {
> -return;
> +if (s->tim_cr1 & TIM_CR1_CMS) {
> +s->count_mode = TIMER_UP_COUNT;
>  }
>
> -DB_PRINT("Alarm set at: 0x%x\n", s->tim_cr1);
> +if (s->tim_cr1 & TIM_CR1_CEN) {
> +DB_PRINT("Enabling timer\n");
> +ptimer_set_freq(s->timer, s->freq_hz);
> +ptimer_run(s->timer, !(s->tim_cr1 & 0x04));
> +} else {
> +DB_PRINT("Disabling timer\n");
> +

Re: [PULL 46/49] target/riscv: Don't assume PMU counters are continuous

[Alistair Francis]

On Fri, Nov 10, 2023 at 1:24 AM Peter Maydell  wrote:
>
> On Tue, 7 Nov 2023 at 02:36, Alistair Francis  wrote:
> >
> > From: Rob Bradford 
> >
> > Check the PMU available bitmask when checking if a counter is valid
> > rather than comparing the index against the number of PMUs.
> >
> > Signed-off-by: Rob Bradford 
> > Reviewed-by: LIU Zhiwei 
> > Reviewed-by: Alistair Francis 
> > Reviewed-by: Atish Patra 
> > Message-ID: <20231031154000.18134-3-rbradf...@rivosinc.com>
> > Signed-off-by: Alistair Francis 
> > ---
> >  target/riscv/csr.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index fc26b52c88..fde7ce1a53 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -188,7 +188,8 @@ static RISCVException zcmt(CPURISCVState *env, int 
> > csrno)
> >  #if !defined(CONFIG_USER_ONLY)
> >  static RISCVException mctr(CPURISCVState *env, int csrno)
> >  {
> > -int pmu_num = riscv_cpu_cfg(env)->pmu_num;
> > +RISCVCPU *cpu = env_archcpu(env);
> > +uint32_t pmu_avail_ctrs = cpu->pmu_avail_ctrs;
> >  int ctr_index;
> >  int base_csrno = CSR_MHPMCOUNTER3;
> >
> > @@ -197,7 +198,7 @@ static RISCVException mctr(CPURISCVState *env, int 
> > csrno)
> >  base_csrno += 0x80;
> >  }
> >  ctr_index = csrno - base_csrno;
> > -if (!pmu_num || ctr_index >= pmu_num) {
> > +if ((BIT(ctr_index) & pmu_avail_ctrs >> 3) == 0) {
> >  /* The PMU is not enabled or counter is out of range */
> >  return RISCV_EXCP_ILLEGAL_INST;
> >  }
>
> Hi; Coverity is not convinced that ctr_index is necessarily
> guaranteed to be within the valid range to be an argument
> to BIT() (eg that it won't be negative). Looking at the
> code as a human I'm pretty unsure too. Could somebody have
> a look at this and maybe improve the readability / add an
> assertion / fix a bug if any ? (CID 1523910)

The code looks ok to me. I have a patch to add an assert to keep Coverity happy.

>
> More generally there are about half a dozen other riscv
> issues in Coverity at the moment, so if somebody who knows
> the riscv code could have a look at them that would be great.

I am happy to look at it. I didn't realise we could all see the
Coverity data. I just requested permission to see the results

Alistair

>
> thanks
> -- PMM

[PULL 5/6] target/riscv/cpu_helper.c: Invalid exception on MMU translation stage

[Alistair Francis]

From: Ivan Klokov 

According to RISCV privileged spec sect. 5.3.2 Virtual Address Translation 
Process
access-fault exceptions may raise only after PMA/PMP check. Current 
implementation
generates an access-fault for mbare mode even if there were no PMA/PMP errors.
This patch removes the erroneous MMU mode check and generates an access-fault
exception based on the pmp_violation flag only.

Fixes: 1448689c7b ("target/riscv: Allow specifying MMU stage")

Signed-off-by: Ivan Klokov 
Reviewed-by: Alistair Francis 
Reviewed-by: Daniel Henrique Barboza 
Message-ID: <20231121071757.7178-2-ivan.klo...@syntacore.com>
Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 30 +++---
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b7af69de53..9ff0952e46 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1143,47 +1143,31 @@ static void raise_mmu_exception(CPURISCVState *env, 
target_ulong address,
 bool two_stage_indirect)
 {
 CPUState *cs = env_cpu(env);
-int page_fault_exceptions, vm;
-uint64_t stap_mode;
-
-if (riscv_cpu_mxl(env) == MXL_RV32) {
-stap_mode = SATP32_MODE;
-} else {
-stap_mode = SATP64_MODE;
-}
-
-if (first_stage) {
-vm = get_field(env->satp, stap_mode);
-} else {
-vm = get_field(env->hgatp, stap_mode);
-}
-
-page_fault_exceptions = vm != VM_1_10_MBARE && !pmp_violation;
 
 switch (access_type) {
 case MMU_INST_FETCH:
 if (env->virt_enabled && !first_stage) {
 cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
 } else {
-cs->exception_index = page_fault_exceptions ?
-RISCV_EXCP_INST_PAGE_FAULT : RISCV_EXCP_INST_ACCESS_FAULT;
+cs->exception_index = pmp_violation ?
+RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
 }
 break;
 case MMU_DATA_LOAD:
 if (two_stage && !first_stage) {
 cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
 } else {
-cs->exception_index = page_fault_exceptions ?
-RISCV_EXCP_LOAD_PAGE_FAULT : RISCV_EXCP_LOAD_ACCESS_FAULT;
+cs->exception_index = pmp_violation ?
+RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
 }
 break;
 case MMU_DATA_STORE:
 if (two_stage && !first_stage) {
 cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
 } else {
-cs->exception_index = page_fault_exceptions ?
-RISCV_EXCP_STORE_PAGE_FAULT :
-RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+cs->exception_index = pmp_violation ?
+RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
+RISCV_EXCP_STORE_PAGE_FAULT;
 }
 break;
 default:
-- 
2.42.0

[PULL 4/6] riscv: Fix SiFive E CLINT clock frequency

[Alistair Francis]

From: Román Cárdenas 

If you check the manual of SiFive E310 
(https://cdn.sparkfun.com/assets/7/f/0/2/7/fe310-g002-manual-v19p05.pdf),
you can see in Figure 1 that the CLINT is connected to the real time clock, 
which also feeds the AON peripheral (they share the same clock).
In page 43, the docs also say that the timer registers of the CLINT count ticks 
from the rtcclk.

I am currently playing with bare metal applications both in QEMU and a physical 
SiFive E310 board and
I confirm that the CLINT clock in the physical board runs at 32.768 kHz.
In QEMU, the same app produces a completely different outcome, as sometimes a 
new CLINT interrupt is triggered before finishing other tasks.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1978

Signed-off-by: Rom\ufffd\ufffdn C\ufffd\ufffdrdenas 
Reviewed-by: Alistair Francis 
Reviewed-by: Daniel Henrique Barboza 
Message-ID: <20231117082840.55705-1-rcardenas@gmail.com>
Signed-off-by: Alistair Francis 
---
 hw/riscv/sifive_e.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 0d37adc542..87d9602383 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -225,7 +225,7 @@ static void sifive_e_soc_realize(DeviceState *dev, Error 
**errp)
 RISCV_ACLINT_SWI_SIZE,
 RISCV_ACLINT_DEFAULT_MTIMER_SIZE, 0, ms->smp.cpus,
 RISCV_ACLINT_DEFAULT_MTIMECMP, RISCV_ACLINT_DEFAULT_MTIME,
-RISCV_ACLINT_DEFAULT_TIMEBASE_FREQ, false);
+SIFIVE_E_LFCLK_DEFAULT_FREQ, false);
 sifive_e_prci_create(memmap[SIFIVE_E_DEV_PRCI].base);
 
 /* AON */
-- 
2.42.0

[PULL 3/6] target/riscv: don't verify ISA compatibility for zicntr and zihpm

[Alistair Francis]

From: Clément Chigot 

The extensions zicntr and zihpm were officially added in the privilege
instruction set specification 1.12. However, QEMU has been implemented
them long before it and thus they are forced to be on during the cpu
initialization to ensure compatibility (see riscv_cpu_init).
riscv_cpu_disable_priv_spec_isa_exts was not updated when the above
behavior was introduced, resulting in these extensions to be disabled
after all.

Signed-off-by: Clément Chigot 
Fixes: c004099330 ("target/riscv: add zicntr extension flag for TCG")
Fixes: 0824121660 ("target/riscv: add zihpm extension flag for TCG")
Reviewed-by: Daniel Henrique Barboza 
Reviewed-by: Alistair Francis 
Message-ID: <20231114123913.536194-1-chi...@adacore.com>
Signed-off-by: Alistair Francis 
---
 target/riscv/tcg/tcg-cpu.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 08adad304d..8a35683a34 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -250,6 +250,15 @@ static void riscv_cpu_disable_priv_spec_isa_exts(RISCVCPU 
*cpu)
 for (edata = isa_edata_arr; edata && edata->name; edata++) {
 if (isa_ext_is_enabled(cpu, edata->ext_enable_offset) &&
 (env->priv_ver < edata->min_version)) {
+/*
+ * These two extensions are always enabled as they were supported
+ * by QEMU before they were added as extensions in the ISA.
+ */
+if (!strcmp(edata->name, "zicntr") ||
+!strcmp(edata->name, "zihpm")) {
+continue;
+}
+
 isa_ext_update_enabled(cpu, edata->ext_enable_offset, false);
 #ifndef CONFIG_USER_ONLY
 warn_report("disabling %s extension for hart 0x" TARGET_FMT_lx
-- 
2.42.0

[PULL 2/6] hw/riscv/virt.c: do create_fdt() earlier, add finalize_fdt()

[Alistair Francis]

From: Daniel Henrique Barboza 

Commit 49554856f0 fixed a problem, where TPM devices were not appearing
in the FDT, by delaying the FDT creation up until virt_machine_done().
This create a side effect (see gitlab #1925) - devices that need access
to the '/chosen' FDT node during realize() stopped working because, at
that point, we don't have a FDT.

This happens because our FDT creation is monolithic, but it doesn't need
to be. We can add the needed FDT components for realize() time and, at
the same time, do another FDT round where we account for dynamic sysbus
devices.  In other words, the problem fixed by 49554856f0 could also be
fixed by postponing only create_fdt_sockets() and its dependencies,
leaving everything else from create_fdt() to be done during init().

Split the FDT creation in two parts:

- create_fdt(), now moved back to virt_machine_init(), will create FDT
  nodes that doesn't depend on additional (dynamic) devices from the
  sysbus;

- a new finalize_fdt() step is added, where create_fdt_sockets() and
  friends is executed, accounting for the dynamic sysbus devices that
  were added during realize().

This will make both use cases happy: TPM devices are still working as
intended, and devices such as 'guest-loader' have a FDT to work on
during realize().

Fixes: 49554856f0 ("riscv: Generate devicetree only after machine 
initialization is complete")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1925
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Alistair Francis 
Message-ID: <20231110172559.73209-1-dbarb...@ventanamicro.com>
Signed-off-by: Alistair Francis 
---
 hw/riscv/virt.c | 71 +
 1 file changed, 42 insertions(+), 29 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index c7fc97e273..d2eac24156 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -962,7 +962,6 @@ static void create_fdt_uart(RISCVVirtState *s, const 
MemMapEntry *memmap,
 qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", UART0_IRQ, 0x4);
 }
 
-qemu_fdt_add_subnode(ms->fdt, "/chosen");
 qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", name);
 g_free(name);
 }
@@ -1023,11 +1022,29 @@ static void create_fdt_fw_cfg(RISCVVirtState *s, const 
MemMapEntry *memmap)
 g_free(nodename);
 }
 
-static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
+static void finalize_fdt(RISCVVirtState *s)
 {
-MachineState *ms = MACHINE(s);
 uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
 uint32_t irq_pcie_phandle = 1, irq_virtio_phandle = 1;
+
+create_fdt_sockets(s, virt_memmap, , _mmio_phandle,
+   _pcie_phandle, _virtio_phandle,
+   _pcie_phandle);
+
+create_fdt_virtio(s, virt_memmap, irq_virtio_phandle);
+
+create_fdt_pcie(s, virt_memmap, irq_pcie_phandle, msi_pcie_phandle);
+
+create_fdt_reset(s, virt_memmap, );
+
+create_fdt_uart(s, virt_memmap, irq_mmio_phandle);
+
+create_fdt_rtc(s, virt_memmap, irq_mmio_phandle);
+}
+
+static void create_fdt(RISCVVirtState *s, const MemMapEntry *memmap)
+{
+MachineState *ms = MACHINE(s);
 uint8_t rng_seed[32];
 
 ms->fdt = create_device_tree(>fdt_size);
@@ -1047,28 +1064,16 @@ static void create_fdt(RISCVVirtState *s, const 
MemMapEntry *memmap)
 qemu_fdt_setprop_cell(ms->fdt, "/soc", "#size-cells", 0x2);
 qemu_fdt_setprop_cell(ms->fdt, "/soc", "#address-cells", 0x2);
 
-create_fdt_sockets(s, memmap, , _mmio_phandle,
-   _pcie_phandle, _virtio_phandle,
-   _pcie_phandle);
-
-create_fdt_virtio(s, memmap, irq_virtio_phandle);
-
-create_fdt_pcie(s, memmap, irq_pcie_phandle, msi_pcie_phandle);
-
-create_fdt_reset(s, memmap, );
-
-create_fdt_uart(s, memmap, irq_mmio_phandle);
-
-create_fdt_rtc(s, memmap, irq_mmio_phandle);
-
-create_fdt_flash(s, memmap);
-create_fdt_fw_cfg(s, memmap);
-create_fdt_pmu(s);
+qemu_fdt_add_subnode(ms->fdt, "/chosen");
 
 /* Pass seed to RNG */
 qemu_guest_getrandom_nofail(rng_seed, sizeof(rng_seed));
 qemu_fdt_setprop(ms->fdt, "/chosen", "rng-seed",
  rng_seed, sizeof(rng_seed));
+
+create_fdt_flash(s, memmap);
+create_fdt_fw_cfg(s, memmap);
+create_fdt_pmu(s);
 }
 
 static inline DeviceState *gpex_pcie_init(MemoryRegion *sys_mem,
@@ -1257,15 +1262,12 @@ static void virt_machine_done(Notifier *notifier, void 
*data)
 uint64_t kernel_entry = 0;
 BlockBackend *pflash_blk0;
 
-/* load/create device tree */
-if (machine->dtb) {
-machine->fdt = load_device_tree(machine->dtb, >fdt_size);
-if (!machine->fdt) {
-error_report("load_device_tree() failed");
-exit(1);
-}
-} else {
-create_fdt(s, memmap);
+/*
+ * An user provided dtb must include everything, including
+ * dynamic sysbus devices. Our FDT needs to be finalized.
+ */
+if 

[PULL 6/6] target/riscv/cpu_helper.c: Fix mxr bit behavior

[Alistair Francis]

From: Ivan Klokov 

According to RISCV Specification sect 9.5 on two stage translation when
V=1 the vsstatus(mstatus in QEMU's terms) field MXR, which makes
execute-only pages readable, only overrides VS-stage page protection.
Setting MXR at HS-level(mstatus_hs), however, overrides both VS-stage
and G-stage execute-only permissions.

The hypervisor extension changes the behavior of MXR\MPV\MPRV bits.
Due to RISCV Specification sect. 9.4.1 when MPRV=1, explicit memory
accesses are translated and protected, and endianness is applied, as
though the current virtualization mode were set to MPV and the current
nominal privilege mode were set to MPP. vsstatus.MXR makes readable
those pages marked executable at the VS translation stage.

Fixes: 36a18664ba ("target/riscv: Implement second stage MMU")

Signed-off-by: Ivan Klokov 
Reviewed-by: Alistair Francis 
Reviewed-by: Daniel Henrique Barboza 
Message-ID: <20231121071757.7178-3-ivan.klo...@syntacore.com>
Signed-off-by: Alistair Francis 
---
 target/riscv/cpu_helper.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9ff0952e46..e7e23b34f4 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1032,13 +1032,29 @@ restart:
 prot |= PAGE_WRITE;
 }
 if (pte & PTE_X) {
-bool mxr;
+bool mxr = false;
 
-if (first_stage == true) {
+/*
+ * Use mstatus for first stage or for the second stage without
+ * virt_enabled (MPRV+MPV)
+ */
+if (first_stage || !env->virt_enabled) {
 mxr = get_field(env->mstatus, MSTATUS_MXR);
-} else {
-mxr = get_field(env->vsstatus, MSTATUS_MXR);
 }
+
+/* MPRV+MPV case, check VSSTATUS */
+if (first_stage && two_stage && !env->virt_enabled) {
+mxr |= get_field(env->vsstatus, MSTATUS_MXR);
+}
+
+/*
+ * Setting MXR at HS-level overrides both VS-stage and G-stage
+ * execute-only permissions
+ */
+if (env->virt_enabled) {
+mxr |= get_field(env->mstatus_hs, MSTATUS_MXR);
+}
+
 if (mxr) {
 prot |= PAGE_READ;
 }
-- 
2.42.0

[PULL 0/6] riscv-to-apply queue

[Alistair Francis]

The following changes since commit 8fa379170c2a12476021f5f50d6cf3f672e79e7b:

  Update version for v8.2.0-rc1 release (2023-11-21 13:56:12 -0500)

are available in the Git repository at:

  https://github.com/alistair23/qemu.git tags/pull-riscv-to-apply-20231122

for you to fetch changes up to 6bca4d7d1ff2b8857486c3ff31f5c6fc3e3984b4:

  target/riscv/cpu_helper.c: Fix mxr bit behavior (2023-11-22 14:03:37 +1000)


Fourth RISC-V PR for 8.2

This is a few bug fixes for the 8.2 release

* Add Zicboz block size to hwprobe
* Creat the virt machine FDT before machine init is complete
* Don't verify ISA compatibility for zicntr and zihpm
* Fix SiFive E CLINT clock frequency
* Fix invalid exception on MMU translation stage
* Fix mxr bit behavior


Clément Chigot (1):
  target/riscv: don't verify ISA compatibility for zicntr and zihpm

Daniel Henrique Barboza (1):
  hw/riscv/virt.c: do create_fdt() earlier, add finalize_fdt()

Ivan Klokov (2):
  target/riscv/cpu_helper.c: Invalid exception on MMU translation stage
  target/riscv/cpu_helper.c: Fix mxr bit behavior

Palmer Dabbelt (1):
  linux-user/riscv: Add Zicboz block size to hwprobe

Román Cárdenas (1):
  riscv: Fix SiFive E CLINT clock frequency

 hw/riscv/sifive_e.c|  2 +-
 hw/riscv/virt.c| 71 +++---
 linux-user/syscall.c   |  6 
 target/riscv/cpu_helper.c  | 54 +--
 target/riscv/tcg/tcg-cpu.c |  9 ++
 5 files changed, 85 insertions(+), 57 deletions(-)

[PULL 1/6] linux-user/riscv: Add Zicboz block size to hwprobe

[Alistair Francis]

From: Palmer Dabbelt 

Support for probing the Zicboz block size landed in Linux 6.6, which was
released a few weeks ago.  This provides the user-configured block size
when Zicboz is enabled.

Signed-off-by: Palmer Dabbelt 
Reviewed-by: Alistair Francis 
Reviewed-by: Daniel Henrique Barboza 
Message-ID: <20231110173716.24423-1-pal...@rivosinc.com>
Signed-off-by: Alistair Francis 
---
 linux-user/syscall.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 16ca5ea7b6..e384e14248 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8808,6 +8808,8 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, 
abi_long count)
 #define RISCV_HWPROBE_MISALIGNED_UNSUPPORTED (4 << 0)
 #define RISCV_HWPROBE_MISALIGNED_MASK(7 << 0)
 
+#define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE 6
+
 struct riscv_hwprobe {
 abi_llong  key;
 abi_ullong value;
@@ -8860,6 +8862,10 @@ static void risc_hwprobe_fill_pairs(CPURISCVState *env,
 case RISCV_HWPROBE_KEY_CPUPERF_0:
 __put_user(RISCV_HWPROBE_MISALIGNED_FAST, >value);
 break;
+case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
+value = cfg->ext_zicboz ? cfg->cboz_blocksize : 0;
+__put_user(value, >value);
+break;
 default:
 __put_user(-1, >key);
 break;
-- 
2.42.0

[PATCH v4 4/4] hw/riscv/virt: Add IOPMP support

[Ethan Chen via]

If a source device is connected to the IOPMP device, its memory access will be
checked by the IOPMP rule.

- Add 'iopmp=on' option to add an iopmp device and a dma device which is
  connected to the iopmp to machine. This option is assumed to be "off"
- Add 'iopmp_cascade=on' option to add second iopmp device which is cascaded by
  first iopmp device to machine. When iopmp option is "off", this option has no
  effect.

Signed-off-by: Ethan Chen 
---
 docs/system/riscv/virt.rst | 11 +++
 hw/riscv/Kconfig   |  2 ++
 hw/riscv/virt.c| 65 ++
 include/hw/riscv/virt.h| 10 +-
 4 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst
index f5fa7b8b29..e1b4aa4f94 100644
--- a/docs/system/riscv/virt.rst
+++ b/docs/system/riscv/virt.rst
@@ -111,6 +111,17 @@ The following machine-specific options are supported:
   having AIA IMSIC (i.e. "aia=aplic-imsic" selected). When not specified,
   the default number of per-HART VS-level AIA IMSIC pages is 0.
 
+- iopmp=[on|off]
+
+  When this option is "on".  An iopmp device and a dma device which is 
connected
+  to the iopmp are added to machine. This option is assumed to be "off".
+
+- iopmp_cascade=[on|off]
+
+  When this option is "on". Second iopmp device which is cascaded by first 
iopmp
+  device is added to machine. When iopmp option is "off", this option has no
+  effect. This option is assumed to be "off".
+
 Running Linux kernel
 
 
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index b6a5eb4452..c30a104aa4 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -45,6 +45,8 @@ config RISCV_VIRT
 select FW_CFG_DMA
 select PLATFORM_BUS
 select ACPI
+select ATCDMAC300
+select RISCV_IOPMP
 
 config SHAKTI_C
 bool
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index c7fc97e273..92b748bfc7 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -53,6 +53,8 @@
 #include "hw/display/ramfb.h"
 #include "hw/acpi/aml-build.h"
 #include "qapi/qapi-visit-common.h"
+#include "hw/misc/riscv_iopmp.h"
+#include "hw/dma/atcdmac300.h"
 
 /*
  * The virt machine physical address space used by some of the devices
@@ -97,6 +99,9 @@ static const MemMapEntry virt_memmap[] = {
 [VIRT_UART0] ={ 0x1000, 0x100 },
 [VIRT_VIRTIO] =   { 0x10001000,0x1000 },
 [VIRT_FW_CFG] =   { 0x1010,  0x18 },
+[VIRT_IOPMP] ={ 0x1020,  0x10 },
+[VIRT_IOPMP2] =   { 0x1030,  0x10 },
+[VIRT_DMAC] = { 0x1040,  0x10 },
 [VIRT_FLASH] ={ 0x2000, 0x400 },
 [VIRT_IMSIC_M] =  { 0x2400, VIRT_IMSIC_MAX_SIZE },
 [VIRT_IMSIC_S] =  { 0x2800, VIRT_IMSIC_MAX_SIZE },
@@ -1534,6 +1539,23 @@ static void virt_machine_init(MachineState *machine)
 sysbus_create_simple("goldfish_rtc", memmap[VIRT_RTC].base,
 qdev_get_gpio_in(mmio_irqchip, RTC_IRQ));
 
+if (s->have_iopmp) {
+/* IOPMP */
+DeviceState *iopmp_dev = iopmp_create(memmap[VIRT_IOPMP].base,
+qdev_get_gpio_in(DEVICE(mmio_irqchip), IOPMP_IRQ));
+/* DMA with IOPMP */
+DeviceState *dmac_dev = atcdmac300_create("atcdmac300",
+memmap[VIRT_DMAC].base, memmap[VIRT_DMAC].size,
+qdev_get_gpio_in(DEVICE(mmio_irqchip), DMAC_IRQ));
+atcdmac300_connect_iopmp(dmac_dev, &(IOPMP(iopmp_dev)->iopmp_as),
+(StreamSink *)&(IOPMP(iopmp_dev)->transaction_info_sink), 0);
+if (s->have_iopmp_cascade) {
+DeviceState *iopmp_dev2 = iopmp_create(memmap[VIRT_IOPMP2].base,
+qdev_get_gpio_in(DEVICE(mmio_irqchip), IOPMP2_IRQ));
+cascade_iopmp(iopmp_dev, iopmp_dev2);
+}
+}
+
 for (i = 0; i < ARRAY_SIZE(s->flash); i++) {
 /* Map legacy -drive if=pflash to machine properties */
 pflash_cfi01_legacy_drive(s->flash[i],
@@ -1628,6 +1650,35 @@ static void virt_set_aclint(Object *obj, bool value, 
Error **errp)
 s->have_aclint = value;
 }
 
+static bool virt_get_iopmp(Object *obj, Error **errp)
+{
+RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
+
+return s->have_iopmp;
+}
+
+static void virt_set_iopmp(Object *obj, bool value, Error **errp)
+{
+RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
+
+s->have_iopmp = value;
+}
+
+static bool virt_get_iopmp_cascade(Object *obj, Error **errp)
+{
+RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
+
+return s->have_iopmp_cascade;
+}
+
+static void virt_set_iopmp_cascade(Object *obj, bool value, Error **errp)
+{
+RISCVVirtState *s = RISCV_VIRT_MACHINE(obj);
+
+s->have_iopmp_cascade = value;
+}
+
+
 bool virt_is_acpi_enabled(RISCVVirtState *s)
 {
 return s->acpi != ON_OFF_AUTO_OFF;
@@ -1730,6 +1781,20 @@ static void virt_machine_class_init(ObjectClass *oc, 
void *data)
   NULL, NULL);
 

[PATCH v4 1/4] hw/core: Add config stream

[Ethan Chen via]

Make other device can use /hw/core/stream.c by select this config.

Signed-off-by: Ethan Chen 
---
 hw/Kconfig  | 1 +
 hw/core/Kconfig | 3 +++
 hw/core/meson.build | 2 +-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/Kconfig b/hw/Kconfig
index 9ca7b38c31..e4d153dce7 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -79,6 +79,7 @@ config XILINX
 config XILINX_AXI
 bool
 select PTIMER # for hw/dma/xilinx_axidma.c
+select STREAM
 
 config XLNX_ZYNQMP
 bool
diff --git a/hw/core/Kconfig b/hw/core/Kconfig
index 9397503656..e89ffa728b 100644
--- a/hw/core/Kconfig
+++ b/hw/core/Kconfig
@@ -27,3 +27,6 @@ config REGISTER
 
 config SPLIT_IRQ
 bool
+
+config STREAM
+bool
diff --git a/hw/core/meson.build b/hw/core/meson.build
index 67dad04de5..0893917b12 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -32,8 +32,8 @@ system_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: 
files('platform-bus.c'))
 system_ss.add(when: 'CONFIG_PTIMER', if_true: files('ptimer.c'))
 system_ss.add(when: 'CONFIG_REGISTER', if_true: files('register.c'))
 system_ss.add(when: 'CONFIG_SPLIT_IRQ', if_true: files('split-irq.c'))
-system_ss.add(when: 'CONFIG_XILINX_AXI', if_true: files('stream.c'))
 system_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c'))
+system_ss.add(when: 'CONFIG_STREAM', if_true: files('stream.c'))
 
 system_ss.add(files(
   'cpu-sysemu.c',
-- 
2.34.1

[PATCH v4 3/4] hw/dma: Add Andes ATCDMAC300 support

[Ethan Chen via]

ATCDMAC300 is a direct memory access controller (DMAC) which transfers data
efficiently between devices on the AMBA AXI4 bus.

ATCDMAC300 supports up to 8 DMA channels. Each DMA channel provides a set of
registers to describe the intended data transfers

To support RISC-V IOPMP, a memory access device needs to
- Support setup the connection to IOPMP
- Support asynchronous I/O to handle stall transactions
- Support transaction information (optional)

To setup the connection to IOPMP, function atcdmac300_connect_iopmp is called.
The iopmp_as and sid are needed, and transaction_info_sink is optional (null if
it is not supported).

To handle IOPMP stall transaction, this device uses asynchronous I/O by doing
memory access in bottom half coroutine. If it receives an IOPMP stall, the
coroutine yields to let the cpu execute then will retry at the bottom half
called next time. You can set the iothread property to make the device run on
iothread.

To send transaction information to IOPMP streamsink, function
transaction_info_push is called before memory access.

Signed-off-by: Ethan Chen 
---
 hw/dma/Kconfig  |   4 +
 hw/dma/atcdmac300.c | 566 
 hw/dma/meson.build  |   1 +
 include/hw/dma/atcdmac300.h | 180 
 4 files changed, 751 insertions(+)
 create mode 100644 hw/dma/atcdmac300.c
 create mode 100644 include/hw/dma/atcdmac300.h

diff --git a/hw/dma/Kconfig b/hw/dma/Kconfig
index 98fbb1bb04..a1d335b52f 100644
--- a/hw/dma/Kconfig
+++ b/hw/dma/Kconfig
@@ -30,3 +30,7 @@ config SIFIVE_PDMA
 config XLNX_CSU_DMA
 bool
 select REGISTER
+
+config ATCDMAC300
+bool
+select STREAM
diff --git a/hw/dma/atcdmac300.c b/hw/dma/atcdmac300.c
new file mode 100644
index 00..7db408aa54
--- /dev/null
+++ b/hw/dma/atcdmac300.c
@@ -0,0 +1,566 @@
+/*
+ * Andes ATCDMAC300 (Andes Technology DMA Controller)
+ *
+ * Copyright (c) 2022 Andes Tech. Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see 
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/dma/atcdmac300.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "exec/memattrs.h"
+#include "exec/address-spaces.h"
+#include "hw/stream.h"
+#include "hw/misc/riscv_iopmp_transaction_info.h"
+
+/* #define DEBUG_ANDES_ATCDMAC300 */
+#define LOGGE(x...) qemu_log_mask(LOG_GUEST_ERROR, x)
+#define xLOG(x...)
+#define yLOG(x...) qemu_log(x)
+#ifdef DEBUG_ANDES_ATCDMAC300
+  #define LOG(x...) yLOG(x)
+#else
+  #define LOG(x...) xLOG(x)
+#endif
+
+#define MEMTX_IOPMP_STALL (1 << 3)
+
+static void atcdmac300_dma_int_stat_update(ATCDMAC300State *s, int status,
+   int ch)
+{
+s->IntStatus |= (1 << (status + ch));
+}
+
+static void atcdmac300_dma_reset_chan(ATCDMAC300State *s, int ch)
+{
+if (s) {
+s->chan[ch].ChnCtrl &= ~(1 << CHAN_CTL_ENABLE);
+s->ChEN &= ~(1 << ch);
+}
+}
+
+static void atcdmac300_dma_reset(ATCDMAC300State *s)
+{
+int ch;
+for (ch = 0; ch < ATCDMAC300_MAX_CHAN; ch++) {
+atcdmac300_dma_reset_chan(s, ch);
+}
+}
+
+static uint64_t atcdmac300_read(void *opaque, hwaddr offset, unsigned size)
+{
+ATCDMAC300State *s = opaque;
+int ch = 0;
+uint64_t result = 0;
+
+if (offset >= 0x40) {
+ch = ATCDMAC300_GET_CHAN(offset);
+offset = ATCDMAC300_GET_OFF(offset, ch);
+}
+
+switch (offset) {
+case ATCDMAC300_DMA_CFG:
+result = s->DMACfg;
+break;
+case ATCDMAC300_DMAC_CTRL:
+break;
+case ATCDMAC300_CHN_ABT:
+break;
+case ATCDMAC300_INT_STATUS:
+result = s->IntStatus;
+break;
+case ATCDMAC300_CHAN_ENABLE:
+result = s->ChEN;
+break;
+case ATCDMAC300_CHAN_CTL:
+result = s->chan[ch].ChnCtrl;
+break;
+default:
+LOGGE("%s: Bad offset 0x%" HWADDR_PRIX "\n",
+  __func__, offset);
+break;
+}
+
+LOG("### atcdmac300_read()=0x%lx, val=0x%lx\n", offset, result);
+return result;
+}
+
+static void transaction_info_push(StreamSink *sink, uint8_t *buf,
+bool eop)
+{
+if (sink == NULL) {
+/* Do nothing if streamsink is not connected */
+return;
+}
+if (eop) {
+while (stream_push(sink, buf, sizeof(iopmp_transaction_info), true)
+  

[PATCH v4 2/4] Add RISC-V IOPMP support

[Ethan Chen via]

Support specification Version 1.0.0-draft4 rapid-k model.
The specification url:
https://github.com/riscv-non-isa/iopmp-spec/blob/main/riscv_iopmp_specification.pdf

IOPMP check memory access from deivce is valid or not. This implementation uses
IOMMU to change address space that device access. There are three possible
results of an access: valid, blocked, and stalled.

If an access is valid, target address spcae is downstream_as(system_memory).
If an access is blocked, it will go to blocked_io_as. The operation of
blocked_io_as could be a bus error, a decode error, or it can respond a success
with fabricated data depending on IOPMP ERRREACT register value.
If an access is stalled, it will go to stall_io_as. The operation of stall_io_as
does nothing but return a stall result to source device. Source device should
retry the access if it gets a stall result.

IOPMP implementation is rely on bus signal. For example IOPMP on AXI bus checks
the AXI burst transaction. A streamsink to receive general transaction_info(sid,
start address, end address) is added to IOPMP.

If the source device support transaction_info, it can first send a
transaction_info to IOPMP streamsink then do the memory access. IOPMP will do
additional partially hit check with transaction info.
If the source device does not support transaction info. IOPMP will not check
partially hit.

Signed-off-by: Ethan Chen 
---
 hw/misc/Kconfig   |   4 +
 hw/misc/meson.build   |   1 +
 hw/misc/riscv_iopmp.c | 966 ++
 include/hw/misc/riscv_iopmp.h | 341 +++
 .../hw/misc/riscv_iopmp_transaction_info.h|  28 +
 5 files changed, 1340 insertions(+)
 create mode 100644 hw/misc/riscv_iopmp.c
 create mode 100644 include/hw/misc/riscv_iopmp.h
 create mode 100644 include/hw/misc/riscv_iopmp_transaction_info.h

diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index cc8a8c1418..953569e682 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -200,4 +200,8 @@ config IOSB
 config XLNX_VERSAL_TRNG
 bool
 
+config RISCV_IOPMP
+bool
+select STREAM
+
 source macio/Kconfig
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 36c20d5637..86b81e1690 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -35,6 +35,7 @@ system_ss.add(when: 'CONFIG_SIFIVE_E_PRCI', if_true: 
files('sifive_e_prci.c'))
 system_ss.add(when: 'CONFIG_SIFIVE_E_AON', if_true: files('sifive_e_aon.c'))
 system_ss.add(when: 'CONFIG_SIFIVE_U_OTP', if_true: files('sifive_u_otp.c'))
 system_ss.add(when: 'CONFIG_SIFIVE_U_PRCI', if_true: files('sifive_u_prci.c'))
+specific_ss.add(when: 'CONFIG_RISCV_IOPMP', if_true: files('riscv_iopmp.c'))
 
 subdir('macio')
 
diff --git a/hw/misc/riscv_iopmp.c b/hw/misc/riscv_iopmp.c
new file mode 100644
index 00..098a3b81fd
--- /dev/null
+++ b/hw/misc/riscv_iopmp.c
@@ -0,0 +1,966 @@
+/*
+ * QEMU RISC-V IOPMP (Input Output Physical Memory Protection)
+ *
+ * Copyright (c) 2023 Andes Tech. Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "trace.h"
+#include "exec/exec-all.h"
+#include "exec/address-spaces.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "hw/misc/riscv_iopmp.h"
+#include "memory.h"
+#include "hw/irq.h"
+
+#define TYPE_IOPMP_IOMMU_MEMORY_REGION "iopmp-iommu-memory-region"
+#define TYPE_IOPMP_TRASACTION_INFO_SINK "iopmp_transaction_info_sink"
+
+DECLARE_INSTANCE_CHECKER(Iopmp_StreamSink, IOPMP_TRASACTION_INFO_SINK,
+ TYPE_IOPMP_TRASACTION_INFO_SINK)
+#define LOGGE(x...) qemu_log_mask(LOG_GUEST_ERROR, x)
+#define xLOG(x...)
+#define yLOG(x...) qemu_log(x)
+#ifdef DEBUG_RISCV_IOPMP
+  #define LOG(x...) yLOG(x)
+#else
+  #define LOG(x...) xLOG(x)
+#endif
+
+#define MEMTX_IOPMP_STALL (1 << 3)
+
+
+static void iopmp_decode_napot(target_ulong a, target_ulong *sa,
+   target_ulong *ea)
+{
+/*
+ * ...aaa0   8-byte NAPOT range
+ * ...aa01   16-byte NAPOT range
+ * ...a011   32-byte NAPOT range
+ * ...
+ * aa01...   2^XLEN-byte NAPOT range
+ * a011...   2^(XLEN+1)-byte NAPOT range
+ * 0111...   2^(XLEN+2)-byte NAPOT range
+ *  ...   Reserved
+ */
+
+a = (a << 2) | 0x3;
+*sa = a & (a + 1);
+*ea = a | (a + 1);
+}
+
+static 

[PATCH v4 0/4] Support RISC-V IOPMP

[Ethan Chen via]

This series implements IOPMP specification v1.0.0-draft4 rapid-k model.
The specification url:
https://github.com/riscv-non-isa/iopmp-spec/blob/main/riscv_iopmp_specification.pdf

When IOPMP is enabled, a DMA device ATCDMAC300 is added to RISC-V virt
platform. This DMA device is connected to the IOPMP and has the functionalities
required by IOPMP, including:
- Support setup the connection to IOPMP
- Support asynchronous I/O to handle stall transactions
- Send transaction information

IOPMP takes a transaction which partially match an entry as a partially hit
error. The transaction size is depending on source device, destination device
and bus.

Source device can send a transaction_info to IOPMP. IOPMP will check partially
hit by transaction_info. If source device does not send a transaction_info,
IOPMP checks information in IOMMU and dose not check partially hit.

Changes for v4:

  - Add descriptions of IOPMP and ATCDMAC300
  - Refine coding style and comments
  - config XILINX_AXI does not include file stream.c but selects config STREAM
instead.
  - ATCDMAC300: INT_STATUS is write 1 clear per bit
Rename iopmp_address_sink to transcation_info_sink
  - IOPMP: Refine error message and remove unused variable
  - VIRT: Document new options
atcdmac300 is only added when iopmp is enabled
  serial setting should not be changed

Ethan Chen (4):
  hw/core: Add config stream
  Add RISC-V IOPMP support
  hw/dma: Add Andes ATCDMAC300 support
  hw/riscv/virt: Add IOPMP support

 docs/system/riscv/virt.rst|  11 +
 hw/Kconfig|   1 +
 hw/core/Kconfig   |   3 +
 hw/core/meson.build   |   2 +-
 hw/dma/Kconfig|   4 +
 hw/dma/atcdmac300.c   | 566 ++
 hw/dma/meson.build|   1 +
 hw/misc/Kconfig   |   4 +
 hw/misc/meson.build   |   1 +
 hw/misc/riscv_iopmp.c | 966 ++
 hw/riscv/Kconfig  |   2 +
 hw/riscv/virt.c   |  65 ++
 include/hw/dma/atcdmac300.h   | 180 
 include/hw/misc/riscv_iopmp.h | 341 +++
 .../hw/misc/riscv_iopmp_transaction_info.h|  28 +
 include/hw/riscv/virt.h   |  10 +-
 16 files changed, 2183 insertions(+), 2 deletions(-)
 create mode 100644 hw/dma/atcdmac300.c
 create mode 100644 hw/misc/riscv_iopmp.c
 create mode 100644 include/hw/dma/atcdmac300.h
 create mode 100644 include/hw/misc/riscv_iopmp.h
 create mode 100644 include/hw/misc/riscv_iopmp_transaction_info.h

-- 
2.34.1

RE: [PATCH v7 00/27] vfio: Adopt iommufd

[Duan, Zhenzhong]

>-Original Message-
>From: Nicolin Chen 
>Sent: Wednesday, November 22, 2023 6:56 AM
>Subject: Re: [PATCH v7 00/27] vfio: Adopt iommufd
>
>On Tue, Nov 21, 2023 at 04:43:59PM +0800, Zhenzhong Duan wrote:
>
>> qemu code:
>https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_cdev_v7
>> Based on vfio-next, commit id: c487fb8a50
>
>I've tested with an aarch64-softmmu build using both legacy VFIO
>passthrough and IOMMUFD+cdev, although this might be similar to
>what Eric tested.
>
>Also, tried rebasing our nesting changes on top and ran some
>2-stage translation sanity using this branch:
>https://github.com/nicolinc/qemu/tree/wip/iommufd_nesting-11212023-cdev-v7
>(Note that the nesting branch is WIP with no stability guarantee)
>
>I'll do more tests with vSVA cases in the next days, yet FWIW:
>
>Tested-by: Nicolin Chen 

Thanks Nicolin.

BRs.
Zhenzhong

RE: [PATCH v7 08/27] vfio/pci: Introduce a vfio pci hot reset interface

[Duan, Zhenzhong]

Hi Philippe,

>-Original Message-
>From: Philippe Mathieu-Daudé 
>Sent: Wednesday, November 22, 2023 2:39 AM
>Subject: Re: [PATCH v7 08/27] vfio/pci: Introduce a vfio pci hot reset 
>interface
>
>Hi Zhenzhong,
>
>On 21/11/23 09:44, Zhenzhong Duan wrote:
>> Legacy vfio pci and iommufd cdev have different process to hot reset
>> vfio device, expand current code to abstract out pci_hot_reset callback
>> for legacy vfio, this same interface will also be used by iommufd
>> cdev vfio device.
>>
>> Rename vfio_pci_hot_reset to vfio_legacy_pci_hot_reset and move it
>> into container.c.
>>
>> vfio_pci_[pre/post]_reset and vfio_pci_host_match are exported so
>> they could be called in legacy and iommufd pci_hot_reset callback.
>>
>> Suggested-by: Cédric Le Goater 
>> Signed-off-by: Zhenzhong Duan 
>> Reviewed-by: Eric Auger 
>> Tested-by: Eric Auger 
>> ---
>>   hw/vfio/pci.h |   3 +
>>   include/hw/vfio/vfio-container-base.h |   3 +
>>   hw/vfio/container.c   | 170 ++
>>   hw/vfio/pci.c | 168 +
>>   4 files changed, 182 insertions(+), 162 deletions(-)
>
>
>> @@ -2485,166 +2485,10 @@ int
>vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice *vdev,
>>
>>   static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
>>   {
>
>> +return ops->pci_hot_reset(vbasedev, single);
>
>At this point vfio_iommufd_ops.pci_hot_reset is NULL.
>Worth checking for non-NULL before calling.

Yes, vfio_iommufd_ops.pci_hot_reset is NULL here. But we could only use
Iommufd backend after:
"[PATCH v7 10/27] vfio/pci: Allow the selection of a given iommu backend"

With "[PATCH v7 09/27] vfio/iommufd: Enable pci hot reset through iommufd cdev 
interface"
set vfio_iommufd_ops.pci_hot_reset.

Looks not an issue for me.

Thanks
Zhenzhong

RE: [PATCH v7 00/27] vfio: Adopt iommufd

[Duan, Zhenzhong]

>-Original Message-
>From: Cédric Le Goater 
>Sent: Wednesday, November 22, 2023 1:23 AM
>Subject: Re: [PATCH v7 00/27] vfio: Adopt iommufd
>
>Hello Zhenzhong
>
>On 11/21/23 09:43, Zhenzhong Duan wrote:
>> Hi,
>>
>> Thanks all for giving guides and comments on previous series, this is
>> the remaining part of the iommufd support.
>>
>> Besides suggested changes in v6, I'd like to highlight two changes
>> for final review:
>> 1. Instantiate can_be_deleted callback to fix race where iommufd object
>> can be deleted before vfio device
>> 2. After careful re-thinking, I'd like to follow Nicolin's suggestion in v5,
>> remove is_ioas check which indeed looks heavy just for tracepoint.
>> In fact we can get corresponding info by looking over trace context.
>>
>> PATCH 1: Introduce iommufd object
>> PATCH 2-9: add IOMMUFD container and cdev support
>> PATCH 10-17: fd passing for cdev and linking to IOMMUFD
>> PATCH 18: make VFIOContainerBase parameter const
>> PATCH 19-21: Compile out for IOMMUFD for arm, s390x and x86
>> PATCH 22-26: vfio device init code cleanup
>> PATCH 27: add iommufd doc
>>
>>
>> We have done wide test with different combinations, e.g:
>> - PCI device were tested
>> - FD passing and hot reset with some trick.
>> - device hotplug test with legacy and iommufd backends
>> - with or without vIOMMU for legacy and iommufd backends
>> - divices linked to different iommufds
>> - VFIO migration with a E800 net card(no dirty sync support) passthrough
>> - platform, ccw and ap were only compile-tested due to environment limit
>> - test mdev pass through with mtty and mix with real device and different BE
>> - test iommufd object hotplug/unplug and mix with vfio device plug/unplug
>>
>> Given some iommufd kernel limitations, the iommufd backend is
>> not yet fully on par with the legacy backend w.r.t. features like:
>> - p2p mappings (you will see related error traces)
>> - dirty page sync
>> - and etc.
>>
>>
>> qemu code:
>https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_cdev_v7
>> Based on vfio-next, commit id: c487fb8a50
>
>The series is pushed on top of vfio-next in the vfio-8.2 tree :
>
>   https://github.com/legoater/qemu/commits/vfio-8.2
>
>with a little extra to deal with a PPC build failure.

Thanks Cédric. That's strange I didn't see this failure on my env
which has CONFIG_VFIO_PCI enabled by default for PPC.

BRs.
Zhenzhong

Re: [PATCH v2 0/2] Fix mmu translation with H extension

[Alistair Francis]

On Tue, Nov 21, 2023 at 5:19 PM Ivan Klokov  wrote:
>
> A series of patches that correct the conversion of virtual addresses
> to physical ones. Correct exception for mbare mode and fix MXR bit
> behavior with MPV\MPRV bits.
> ---
> v2:
>- Fix typo, specify the fixed commits
> ---
>
> Ivan Klokov (2):
>   target/riscv/cpu_helper.c: Invalid exception on MMU translation stage
>   target/riscv/cpu_helper.c: Fix mxr bit behavior

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/cpu_helper.c | 54 +++
>  1 file changed, 27 insertions(+), 27 deletions(-)
>
> --
> 2.34.1
>
>

[ANNOUNCE] QEMU 8.2.0-rc1 is now available

[Michael Roth]

Hello,

On behalf of the QEMU Team, I'd like to announce the availability of the
second release candidate for the QEMU 8.2 release. This release is meant
for testing purposes and should not be used in a production environment.

  http://download.qemu.org/qemu-8.2.0-rc1.tar.xz
  http://download.qemu.org/qemu-8.2.0-rc1.tar.xz.sig

You can help improve the quality of the QEMU 8.2 release by testing this
release and reporting bugs using our GitLab issue tracker:

  https://gitlab.com/qemu-project/qemu/-/milestones/10

The release plan, as well a documented known issues for release
candidates, are available at:

  http://wiki.qemu.org/Planning/8.2

Please add entries to the ChangeLog for the 8.2 release below:

  http://wiki.qemu.org/ChangeLog/8.2

Thank you to everyone involved!

Changes since rc0:

8fa379170c: Update version for v8.2.0-rc1 release (Stefan Hajnoczi)
ea6a33e776: Revert "tests/avocado: Enable reverse_debugging.py tests in gitlab 
CI" (Thomas Huth)
82d70a84c8: linux-user: Fix loaddr computation for some elf files (Richard 
Henderson)
debb491166: hw/ide/via: implement legacy/native mode switching (Mark 
Cave-Ayland)
7a9d672b81: ide/via: don't attempt to set default BAR addresses (Mark 
Cave-Ayland)
fd6a543d19: ide/pci: introduce pci_ide_update_mode() function (Mark Cave-Ayland)
7552488444: ide/ioport: move ide_portio_list[] and ide_portio_list2[] 
definitions to IDE core (Mark Cave-Ayland)
8f37120383: iotests: Test two stream jobs in a single iothread (Kevin Wolf)
1dbc7d3442: stream: Fix AioContext locking during bdrv_graph_wrlock() (Kevin 
Wolf)
6bc0bcc89f: block: Fix deadlocks in bdrv_graph_wrunlock() (Kevin Wolf)
bb092d6d8f: block: Fix bdrv_graph_wrlock() call in blk_remove_bs() (Kevin Wolf)
eabb921250: hw/ide/ahci: fix legacy software reset (Niklas Cassel)
6f7997e004: hw/xen: clean up xen_block_find_free_vdev() to avoid Coverity false 
positive (David Woodhouse)
87bfffdf75: vl: disable default serial when xen-console is enabled (David 
Woodhouse)
e0c58720bf: ui/pixman-minimal.h: fix empty allocation (Manos Pitsidianakis)
ff2a5bed5f: vl: add missing display_remote++ (Marc-André Lureau)
0e8823072e: ui/console: fix default VC when there are no display (Marc-André 
Lureau)
b7f1bb38b0: ui: use "vc" chardev for dbus, gtk & spice-app (Marc-André Lureau)
69562648f9: vl: revert behaviour for -display none (Marc-André Lureau)
84f85eb95f: net: do not delete nics in net_cleanup() (David Woodhouse)
9050f976e4: net: Update MemReentrancyGuard for NIC (Akihiko Odaki)
7d0fefdf81: net: Provide MemReentrancyGuard * to qemu_new_nic() (Akihiko Odaki)
b664466d8f: ppc/pnv: Fix PNV I2C invalid status after reset (Glenn Miles)
47dfdd238d: ppc/pnv: PNV I2C engines assigned incorrect XSCOM addresses (Glenn 
Miles)
d18b065286: target/ppc: Fix bugs in VSX_CVT_FP_TO_INT and VSX_CVT_FP_TO_INT2 
macros (John Platts)
8bc5ae046d: ppc/pnv: Fix potential overflow in I2C model (Cédric Le Goater)
85d57a37be: tcg/loongarch64: Fix tcg_out_mov() Aborted (Song Gao)
0cbb56c236: hw/arm/fsl-imx: Do not ignore Error argument (Philippe 
Mathieu-Daudé)
d652866007: hw/arm/stm32f100: Report error when incorrect CPU is used (Philippe 
Mathieu-Daudé)
ff6cda35f1: hw/arm/stm32f205: Report error when incorrect CPU is used (Philippe 
Mathieu-Daudé)
e1b72c55b1: hw/arm/stm32f405: Report error when incorrect CPU is used (Philippe 
Mathieu-Daudé)
790a4428f2: hw/core/machine: Constify MachineClass::valid_cpu_types[] (Gavin 
Shan)
3efd849573: target/arm: Fix SME FMOPA (16-bit), BFMOPA (Richard Henderson)
70726a15bc: hw/intc/arm_gicv3: ICC_PMR_EL1 high bits should be RAZ (Ben Dooks)
e867a1242e: target/arm: enable FEAT_RNG on Neoverse-N2 (Marcin Juszkiewicz)
1b173d0606: linux-user: xtensa: fix signal delivery in FDPIC (Max Filippov)
2f926bfd5b: disas/hppa: Show hexcode of instruction along with disassembly 
(Helge Deller)
a01491a238: target/hppa: Fix 64-bit SHRPD instruction (Helge Deller)
298d8b1220: target/i386/cpu: Improve error message for property "vendor" 
(Markus Armbruster)
eeef44b3a5: balloon: Fix a misleading error message (Markus Armbruster)
0a4a1512e0: net: Fix a misleading error message (Markus Armbruster)
517b0220ef: ui/qmp-cmds: Improve two error messages (Markus Armbruster)
b665165938: qga: Improve guest-exec-status error message (Markus Armbruster)
7200fb211e: hmp: Improve sync-profile error message (Markus Armbruster)
db8227a68a: spapr/pci: Correct "does not support hotplugging error messages 
(Markus Armbruster)
c4d74ab24a: tests/avocado: Enable reverse_debugging.py tests in gitlab CI 
(Nicholas Piggin)
cd43f00524: tests/avocado: reverse_debugging drain console to prevent hang 
(Nicholas Piggin)
572960cb23: tests/avocado/mem-addr-space-check: Replace assertEquals() for 
Python 3.12 (Thomas Huth)
3365f3d99a: tests/avocado/replay_kernel: Mark the test_x86_64_pc as flaky 
(Thomas Huth)
9d72dd100c: tests/avocado: Make fetch_asset() unconditionally require a crypto 
hash (Philippe Mathieu-Daudé)
cfe52e91c4: tests/avocado/multiprocess: Add 

Re: [PATCH v2 2/2] target/riscv/cpu_helper.c: Fix mxr bit behavior

[Alistair Francis]

On Tue, Nov 21, 2023 at 6:53 PM Ivan Klokov  wrote:
>
> According to RISCV Specification sect 9.5 on two stage translation when
> V=1 the vsstatus(mstatus in QEMU's terms) field MXR, which makes
> execute-only pages readable, only overrides VS-stage page protection.
> Setting MXR at HS-level(mstatus_hs), however, overrides both VS-stage
> and G-stage execute-only permissions.
>
> The hypervisor extension changes the behavior of MXR\MPV\MPRV bits.
> Due to RISCV Specification sect. 9.4.1 when MPRV=1, explicit memory
> accesses are translated and protected, and endianness is applied, as
> though the current virtualization mode were set to MPV and the current
> nominal privilege mode were set to MPP. vsstatus.MXR makes readable
> those pages marked executable at the VS translation stage.
>
> Fixes: 36a18664ba ("target/riscv: Implement second stage MMU")
>
> Signed-off-by: Ivan Klokov 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu_helper.c | 24 
>  1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9ff0952e46..e7e23b34f4 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1032,13 +1032,29 @@ restart:
>  prot |= PAGE_WRITE;
>  }
>  if (pte & PTE_X) {
> -bool mxr;
> +bool mxr = false;
>
> -if (first_stage == true) {
> +/*
> + * Use mstatus for first stage or for the second stage without
> + * virt_enabled (MPRV+MPV)
> + */
> +if (first_stage || !env->virt_enabled) {
>  mxr = get_field(env->mstatus, MSTATUS_MXR);
> -} else {
> -mxr = get_field(env->vsstatus, MSTATUS_MXR);
>  }
> +
> +/* MPRV+MPV case, check VSSTATUS */
> +if (first_stage && two_stage && !env->virt_enabled) {
> +mxr |= get_field(env->vsstatus, MSTATUS_MXR);
> +}
> +
> +/*
> + * Setting MXR at HS-level overrides both VS-stage and G-stage
> + * execute-only permissions
> + */
> +if (env->virt_enabled) {
> +mxr |= get_field(env->mstatus_hs, MSTATUS_MXR);
> +}
> +
>  if (mxr) {
>  prot |= PAGE_READ;
>  }
> --
> 2.34.1
>
>

Re: [PATCH v2 1/2] target/riscv/cpu_helper.c: Invalid exception on MMU translation stage

[Alistair Francis]

On Tue, Nov 21, 2023 at 6:51 PM Ivan Klokov  wrote:
>
> According to RISCV privileged spec sect. 5.3.2 Virtual Address Translation 
> Process
> access-fault exceptions may raise only after PMA/PMP check. Current 
> implementation
> generates an access-fault for mbare mode even if there were no PMA/PMP errors.
> This patch removes the erroneous MMU mode check and generates an access-fault
> exception based on the pmp_violation flag only.
>
> Fixes: 1448689c7b ("target/riscv: Allow specifying MMU stage")
>
> Signed-off-by: Ivan Klokov 

Please keep existing tags when sending a new version if there aren't any changes

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu_helper.c | 30 +++---
>  1 file changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index b7af69de53..9ff0952e46 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1143,47 +1143,31 @@ static void raise_mmu_exception(CPURISCVState *env, 
> target_ulong address,
>  bool two_stage_indirect)
>  {
>  CPUState *cs = env_cpu(env);
> -int page_fault_exceptions, vm;
> -uint64_t stap_mode;
> -
> -if (riscv_cpu_mxl(env) == MXL_RV32) {
> -stap_mode = SATP32_MODE;
> -} else {
> -stap_mode = SATP64_MODE;
> -}
> -
> -if (first_stage) {
> -vm = get_field(env->satp, stap_mode);
> -} else {
> -vm = get_field(env->hgatp, stap_mode);
> -}
> -
> -page_fault_exceptions = vm != VM_1_10_MBARE && !pmp_violation;
>
>  switch (access_type) {
>  case MMU_INST_FETCH:
>  if (env->virt_enabled && !first_stage) {
>  cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
>  } else {
> -cs->exception_index = page_fault_exceptions ?
> -RISCV_EXCP_INST_PAGE_FAULT : RISCV_EXCP_INST_ACCESS_FAULT;
> +cs->exception_index = pmp_violation ?
> +RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
>  }
>  break;
>  case MMU_DATA_LOAD:
>  if (two_stage && !first_stage) {
>  cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
>  } else {
> -cs->exception_index = page_fault_exceptions ?
> -RISCV_EXCP_LOAD_PAGE_FAULT : RISCV_EXCP_LOAD_ACCESS_FAULT;
> +cs->exception_index = pmp_violation ?
> +RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
>  }
>  break;
>  case MMU_DATA_STORE:
>  if (two_stage && !first_stage) {
>  cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
>  } else {
> -cs->exception_index = page_fault_exceptions ?
> -RISCV_EXCP_STORE_PAGE_FAULT :
> -RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> +cs->exception_index = pmp_violation ?
> +RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
> +RISCV_EXCP_STORE_PAGE_FAULT;
>  }
>  break;
>  default:
> --
> 2.34.1
>
>

Re: [PATCH] hw/timer/hpet: Convert DPRINTF to trace events

[Dan Hoffman]

bump

On Sat, Nov 18, 2023 at 5:13 PM Daniel Hoffman  wrote:
>
> This conversion is pretty straight-forward. Standardized some formatting
> so the +0 and +4 offset cases can recycle the same message.
>
> Signed-off-by: Daniel Hoffman 
> ---
>  hw/timer/hpet.c   | 55 +--
>  hw/timer/trace-events | 16 +
>  2 files changed, 38 insertions(+), 33 deletions(-)
>
> diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
> index 6998094233a..f9d248b4cf7 100644
> --- a/hw/timer/hpet.c
> +++ b/hw/timer/hpet.c
> @@ -39,13 +39,7 @@
>  #include "hw/timer/i8254.h"
>  #include "exec/address-spaces.h"
>  #include "qom/object.h"
> -
> -//#define HPET_DEBUG
> -#ifdef HPET_DEBUG
> -#define DPRINTF printf
> -#else
> -#define DPRINTF(...)
> -#endif
> +#include "trace.h"
>
>  #define HPET_MSI_SUPPORT0
>
> @@ -431,7 +425,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
>  HPETState *s = opaque;
>  uint64_t cur_tick, index;
>
> -DPRINTF("qemu: Enter hpet_ram_readl at %" PRIx64 "\n", addr);
> +trace_hpet_ram_read(addr);
>  index = addr;
>  /*address range of all TN regs*/
>  if (index >= 0x100 && index <= 0x3ff) {
> @@ -439,7 +433,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
>  HPETTimer *timer = >timer[timer_id];
>
>  if (timer_id > s->num_timers) {
> -DPRINTF("qemu: timer id out of range\n");
> +trace_hpet_timer_id_out_of_range(timer_id);
>  return 0;
>  }
>
> @@ -457,7 +451,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
>  case HPET_TN_ROUTE + 4:
>  return timer->fsb >> 32;
>  default:
> -DPRINTF("qemu: invalid hpet_ram_readl\n");
> +trace_hpet_ram_read_invalid();
>  break;
>  }
>  } else {
> @@ -469,7 +463,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
>  case HPET_CFG:
>  return s->config;
>  case HPET_CFG + 4:
> -DPRINTF("qemu: invalid HPET_CFG + 4 hpet_ram_readl\n");
> +trace_hpet_invalid_hpet_cfg(4);
>  return 0;
>  case HPET_COUNTER:
>  if (hpet_enabled(s)) {
> @@ -477,7 +471,7 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
>  } else {
>  cur_tick = s->hpet_counter;
>  }
> -DPRINTF("qemu: reading counter  = %" PRIx64 "\n", cur_tick);
> +trace_hpet_ram_read_reading_counter(0, cur_tick);
>  return cur_tick;
>  case HPET_COUNTER + 4:
>  if (hpet_enabled(s)) {
> @@ -485,12 +479,12 @@ static uint64_t hpet_ram_read(void *opaque, hwaddr addr,
>  } else {
>  cur_tick = s->hpet_counter;
>  }
> -DPRINTF("qemu: reading counter + 4  = %" PRIx64 "\n", cur_tick);
> +trace_hpet_ram_read_reading_counter(4, cur_tick);
>  return cur_tick >> 32;
>  case HPET_STATUS:
>  return s->isr;
>  default:
> -DPRINTF("qemu: invalid hpet_ram_readl\n");
> +trace_hpet_ram_read_invalid();
>  break;
>  }
>  }
> @@ -504,8 +498,7 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
>  HPETState *s = opaque;
>  uint64_t old_val, new_val, val, index;
>
> -DPRINTF("qemu: Enter hpet_ram_writel at %" PRIx64 " = 0x%" PRIx64 "\n",
> -addr, value);
> +trace_hpet_ram_write(addr, value);
>  index = addr;
>  old_val = hpet_ram_read(opaque, addr, 4);
>  new_val = value;
> @@ -515,14 +508,14 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
>  uint8_t timer_id = (addr - 0x100) / 0x20;
>  HPETTimer *timer = >timer[timer_id];
>
> -DPRINTF("qemu: hpet_ram_writel timer_id = 0x%x\n", timer_id);
> +trace_hpet_ram_write_timer_id(timer_id);
>  if (timer_id > s->num_timers) {
> -DPRINTF("qemu: timer id out of range\n");
> +trace_hpet_timer_id_out_of_range(timer_id);
>  return;
>  }
>  switch ((addr - 0x100) % 0x20) {
>  case HPET_TN_CFG:
> -DPRINTF("qemu: hpet_ram_writel HPET_TN_CFG\n");
> +trace_hpet_ram_write_tn_cfg();
>  if (activating_bit(old_val, new_val, HPET_TN_FSB_ENABLE)) {
>  update_irq(timer, 0);
>  }
> @@ -540,10 +533,10 @@ static void hpet_ram_write(void *opaque, hwaddr addr,
>  }
>  break;
>  case HPET_TN_CFG + 4: // Interrupt capabilities
> -DPRINTF("qemu: invalid HPET_TN_CFG+4 write\n");
> +trace_hpet_ram_write_invalid_tn_cfg(4);
>  break;
>  case HPET_TN_CMP: // comparator register
> -DPRINTF("qemu: hpet_ram_writel HPET_TN_CMP\n");
> +trace_hpet_ram_write_tn_cmp(0);
>  if (timer->config & HPET_TN_32BIT) {
>  

Re: [PATCH v3] hw/i386: fix short-circuit logic with non-optimizing builds

[Dan Hoffman]

>From their Discord server in reply to a question about whether such a
patch would be upstreamed: "I suspect this only works in gcc -O0
because of its AST-level "fold", which clang intentionally doesn't
implement. So probably not."

I hope that's enough information to resolve this patch. If any of you
need anything else, please let me know.

On Tue, Nov 21, 2023 at 12:28 PM Dan Hoffman  wrote:
>
> I'm writing a patch to clang's constant folding to address this case
> (doesn't seem too difficult). I'll either follow up with a link to
> some submissions I've made or a bug report on the project describing
> the issue.
>
>
>
> On Tue, Nov 21, 2023 at 10:15 AM Eric Blake  wrote:
> >
> > On Mon, Nov 20, 2023 at 11:20:52AM +0100, Philippe Mathieu-Daudé wrote:
> > > (Cc'ing Eric)
> > >
> > > On 20/11/23 10:28, Michael S. Tsirkin wrote:
> > > > On Sun, Nov 19, 2023 at 07:34:58PM -0600, Dan Hoffman wrote:
> > > > > As far as I can tell, yes. Any optimization level above O0 does not 
> > > > > have this
> > > > > issue (on this version of Clang, at least)
> > > >
> > > > Aha, this is with -O0. That makes sense.
> > >
> > > But then, why the other cases aren't problematic?
> > >
> > > $ git grep -E ' (&&|\|\|) !?kvm_enabled'
> > > hw/arm/boot.c:1228:assert(!(info->secure_board_setup && 
> > > kvm_enabled()));
> >
> > This one's obvious; no kvm_*() calls in the assert.
> >
> > > hw/i386/microvm.c:270:(mms->rtc == ON_OFF_AUTO_AUTO &&
> > > !kvm_enabled())) {
> >
> > Ones like this require reading context to see whether the if() block
> > guarded any kvm_*() calls for the linker to elide - but still a fairly
> > easy audit.
> >
> > > > >
> > > > >  I'm surprised the order of conditions matters for code elision...
> > > > >
> > > > >  > Signed-off-by: Daniel Hoffman 
> > > > >  > ---
> > > > >  >   hw/i386/x86.c | 15 ---
> > > > >  >   1 file changed, 12 insertions(+), 3 deletions(-)
> > > > >  >
> > > > >  > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > > > >  > index b3d054889bb..2b6291ad8d5 100644
> > > > >  > --- a/hw/i386/x86.c
> > > > >  > +++ b/hw/i386/x86.c
> > > > >  > @@ -131,8 +131,12 @@ void x86_cpus_init(X86MachineState 
> > > > > *x86ms, int
> > > > >  default_cpu_version)
> > > > >  >   /*
> > > > >  >* Can we support APIC ID 255 or higher?  With KVM, that 
> > > > > requires
> > > > >  >* both in-kernel lapic and X2APIC userspace API.
> > > > >  > + *
> > > > >  > + * kvm_enabled() must go first to ensure that kvm_* 
> > > > > references are
> > > > >  > + * not emitted for the linker to consume (kvm_enabled() is
> > > > >  > + * a literal `0` in configurations where kvm_* aren't 
> > > > > defined)
> > > > >  >*/
> > > > >  > -if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
> > > > >  > +if (kvm_enabled() && x86ms->apic_id_limit > 255 &&
> > > > >  >   (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
> >
> > Indeed, if clang -O0 treats 'if (cond1 && 0 && cond2)' differently
> > than 'if (0 && cond1 && cond2)' for purposes of eliding the code for
> > cond2, that seems like a blatant missed optimization that we should be
> > reporting to the clang folks.
> >
> > While this patch may solve the immediate issue, it does not scale - if
> > we ever have two separate guards that can both be independently
> > hard-coded to 0 based on configure-time decisions, but which are both
> > used as guards in the same expression, it will become impossible to
> > avoid a '(cond1 && 0 && cond2)' scenario across all 4 possible
> > configurations of those two guards.
> >
> > I have no objection to the patch, but it would be nice if the commit
> > message could point to a clang bug report, if one has been filed.
> >
> > --
> > Eric Blake, Principal Software Engineer
> > Red Hat, Inc.
> > Virtualization:  qemu.org | libguestfs.org
> >

Re: [PATCH for-9.0] hw: Add compat machines for 9.0

[Gavin Shan]

On 11/20/23 19:42, Cornelia Huck wrote:

Add 9.0 machine types for arm/i440fx/m68k/q35/s390x/spapr.

Signed-off-by: Cornelia Huck 
---
  hw/arm/virt.c  |  9 -
  hw/core/machine.c  |  3 +++
  hw/i386/pc.c   |  3 +++
  hw/i386/pc_piix.c  | 17 ++---
  hw/i386/pc_q35.c   | 13 -
  hw/m68k/virt.c |  9 -
  hw/ppc/spapr.c | 15 +--
  hw/s390x/s390-virtio-ccw.c | 14 +-
  include/hw/boards.h|  3 +++
  include/hw/i386/pc.h   |  3 +++
  10 files changed, 80 insertions(+), 9 deletions(-)



For hw/arm/virt.c

Reviewed-by: Gavin Shan 

Re: [PATCH v7 00/27] vfio: Adopt iommufd

[Nicolin Chen]

On Tue, Nov 21, 2023 at 04:43:59PM +0800, Zhenzhong Duan wrote:
 
> qemu code: https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_cdev_v7
> Based on vfio-next, commit id: c487fb8a50

I've tested with an aarch64-softmmu build using both legacy VFIO
passthrough and IOMMUFD+cdev, although this might be similar to
what Eric tested.

Also, tried rebasing our nesting changes on top and ran some
2-stage translation sanity using this branch:
https://github.com/nicolinc/qemu/tree/wip/iommufd_nesting-11212023-cdev-v7
(Note that the nesting branch is WIP with no stability guarantee)

I'll do more tests with vSVA cases in the next days, yet FWIW:

Tested-by: Nicolin Chen 

Re: [PATCH V5 02/12] cpus: stop vm in suspended state

[Peter Xu]

On Tue, Nov 21, 2023 at 04:21:18PM -0500, Steven Sistare wrote:
> On 11/20/2023 4:44 PM, Peter Xu wrote:
> > On Mon, Nov 20, 2023 at 03:55:54PM -0500, Steven Sistare wrote:
> >> If we drop force, then all calls to vm_stop will completely stop the
> >> suspended state, eg an hmp "stop" command. This causes two problems.
> >> First, that is a change in user-visible behavior for something that
> >> currently works,
> > 
> > IMHO it depends on what should be the correct behavior.  IOW, when VM is in
> > SUSPENDED state and then the user sends "stop" QMP command, what should we
> > expect?
> > 
> > My understanding is we should expect to fully stop the VM, including the
> > ticks, for example.  Keeping the ticks running even after QMP "stop"
> > doesn't sound right, isn't it?
> 
> I agree, but others may not, and this decision would require approval from 
> maintainers in other areas, including upper layers such as libvirt that are
> aware of the suspended state.  It is a user-visible change, and may require 
> a new libvirt release.

$ ./scripts/get_maintainer.pl -f system/cpus.c
Richard Henderson  (maintainer:Overall TCG CPUs)
Paolo Bonzini  (reviewer:Overall TCG CPUs)
qemu-devel@nongnu.org (open list:All patches CC here)

I'm also copying Richard, while Dan/Paolo is already in the loop, so we
should have the "quorum" already.  Let's see whether we can already get
some comments from the maintainers..

> 
> >> vs the migration code where we are fixing brokenness.
> > 
> > This is not a migration-only bug if above holds, IMO.
> > 
> >> Second, it does not quite work, because the state becomes
> >> RUN_STATE_PAUSED, so the suspended state is forgotten, and the hmp "cont"
> >> will try to set the running state.  I could fix that by introducing a new
> >> state RUN_STATE_SUSPENDED_STOPPED, but again it is a user-visible change
> >> in existing behavior.  (I even implemented that while developing, then I
> >> realized it was not needed to fix the migration bugs.)
> > 
> > Good point.
> > 
> > Now with above comments, what's your thoughts on whether we should change
> > the user behavior?  My answer is still a yes.
> > 
> > Maybe SUSPENDED should not be a RunState at all? SUSPENDED is guest visible
> > behavior, while something like QMP "stop" is not guest visible.  Maybe we
> > should remember it separately?
> >
> > It means qemu_system_suspend() could remember that in a separate field (as
> > part of guest state), then when wakeup we should conditionally go back
> > with/without vcpus running depending on the new "suspended" state.
>
> Regardless of how we remember it, the status command must still expose
> the suspended state to the user.  The user must be able to see that a
> guest is suspended, and decide when to issue a wakeup command.

Hmm, right, we may want to keep having the SUSPENDED state in RunState,
even another separate "vm_suspended" boolean might still be required.

> 
> If we change the stop command to completely stop a suspended vm, then we must
> allow the user to query whether a vm is suspended-running or 
> suspended-stopped,
> because the command they must issue to resume is different: wakeup for the
> former, and cont for the latter.

If it's stopped, the user must need a "cont" anyway.  And then if after
"cont" the user still sees it's suspended, then would "system_wakeup" work
here if necessary, after that "cont"?

Let's consider the current QEMU with below sequence of operations:

  1) vm running
  2) guest triggers ACPI suspend -> vm suspended
  3) admin triggers "stop" cmd -> vm suspended (ignored..)
  4) admin triggers "cont" cmd -> vm suspended (ignored.. too)

AFAICT both 2) and 3) are unwanted behavior, and after noticing 3) I feel
stronger that this is not a migration issue alone.

It also means after step 1)-3) if we got a wakeup elsewhere, the VM can
actually be running!  That's definitely unexpected after admin sends "stop"
already.  Isn't that another real bug?

I'm slightly confused on why you said above that libvirt will need a new
release. Could you elaborate?  Especially on what scenario we need to
maintain compatibility that still makes sense.

> 
> This change is visible to libvirt.  Adding it will delay this entire patch
> series, and is not necessary for fixing the migration bug.  There is no
> downside to drawing the line here for this series, and possibly changing stop
> semantics in the future.

This series will need to wait for rc releases anyway until 8.2 all out:

  https://wiki.qemu.org/Planning/8.2

I think we still have time to even catch the earliest train right after 8.2
released, if we can reach a consensus soon in whatever form.

Having a partial solution merged for migration is probably doable, but that
will make the code even more complicated and harder to maintain.  So before
doing so, I'd at least like to understand better on what use case you were
describing that will start to fall apart.

Thanks,

-- 
Peter Xu

[PULL 0/1] Seabios hppa v13 patches

[deller]

From: Helge Deller 

The following changes since commit ea6a33e776f0a4bda94460ab0945d953fc801dd1:

  Revert "tests/avocado: Enable reverse_debugging.py tests in gitlab CI" 
(2023-11-21 10:28:55 -0500)

are available in the Git repository at:

  https://github.com/hdeller/qemu-hppa.git tags/seabios-hppa-v13-pull-request

for you to fetch changes up to 69c224816eeb6c760bb1e73073da03a19368df0e:

  target/hppa: Update SeaBIOS-hppa to version 13 (2023-11-21 21:23:03 +0100)


SeaBIOS-hppa v13

Please pull an update of SeaBIOS-hppa to v13 to fix
a system reboot crash in qemu-system-hppa as reported in
https://gitlab.com/qemu-project/qemu/-/issues/1991



Helge Deller (1):
  target/hppa: Update SeaBIOS-hppa to version 13

 pc-bios/hppa-firmware.img | Bin 681332 -> 681388 bytes
 roms/seabios-hppa |   2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)

-- 
2.41.0

Re: [PULL 0/1] linux-user late fix

[Stefan Hajnoczi]

Applied, thanks.

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


signature.asc
Description: PGP signature

[PATCH v2 6/6] xen_arm: Add virtual PCIe host bridge support

[Volodymyr Babchuk]

From: Oleksandr Tyshchenko 

The bridge is needed for virtio-pci support, as QEMU can emulate the
whole bridge with any virtio-pci devices connected to it.

This patch provides a flexible way to configure PCIe brige resources
with xenstore. We made this for several reasons:

- We don't want to clash with vPCI devices, so we need information
  from Xen toolstack on which PCI bus to use.
- The guest memory layout that describes these resources is not stable
  and may vary between guests, so we cannot rely on static resources
  to be always the same for both ends.
- Also the device-models which run in different domains and serve
  virtio-pci devices for the same guest should use different host
  bridge resources for Xen to distinguish. The rule for the guest
  device-tree generation is one PCI host bridge per backend domain.

Signed-off-by: Oleksandr Tyshchenko 
Signed-off-by: Volodymyr Babchuk 

---

Changes from v1:

 - Renamed virtio_pci_host to pcie_host entries in XenStore, because
 there is nothing specific to virtio-pci: any PCI device can be
 emulated via this newly created bridge.
---
 hw/arm/xen_arm.c| 186 
 hw/xen/xen-hvm-common.c |   9 +-
 include/hw/xen/xen_native.h |   8 +-
 3 files changed, 200 insertions(+), 3 deletions(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index b9c3ae14b6..d506d55d0f 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -22,6 +22,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "qapi/qapi-commands-migration.h"
 #include "qapi/visitor.h"
@@ -34,6 +35,9 @@
 #include "hw/xen/xen-hvm-common.h"
 #include "sysemu/tpm.h"
 #include "hw/xen/arch_hvm.h"
+#include "exec/address-spaces.h"
+#include "hw/pci-host/gpex.h"
+#include "hw/virtio/virtio-pci.h"
 
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
@@ -58,6 +62,11 @@ struct XenArmState {
 struct {
 uint64_t tpm_base_addr;
 } cfg;
+
+MemMapEntry pcie_mmio;
+MemMapEntry pcie_ecam;
+MemMapEntry pcie_mmio_high;
+int pcie_irq;
 };
 
 static MemoryRegion ram_lo, ram_hi;
@@ -73,6 +82,7 @@ static MemoryRegion ram_lo, ram_hi;
 #define NR_VIRTIO_MMIO_DEVICES   \
(GUEST_VIRTIO_MMIO_SPI_LAST - GUEST_VIRTIO_MMIO_SPI_FIRST)
 
+/* TODO It should be xendevicemodel_set_pci_intx_level() for PCI interrupts. */
 static void xen_set_irq(void *opaque, int irq, int level)
 {
 if (xendevicemodel_set_irq_level(xen_dmod, xen_domid, irq, level)) {
@@ -129,6 +139,176 @@ static void xen_init_ram(MachineState *machine)
 }
 }
 
+static void xen_create_pcie(XenArmState *xam)
+{
+MemoryRegion *mmio_alias, *mmio_alias_high, *mmio_reg;
+MemoryRegion *ecam_alias, *ecam_reg;
+DeviceState *dev;
+int i;
+
+dev = qdev_new(TYPE_GPEX_HOST);
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
+
+/* Map ECAM space */
+ecam_alias = g_new0(MemoryRegion, 1);
+ecam_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 0);
+memory_region_init_alias(ecam_alias, OBJECT(dev), "pcie-ecam",
+ ecam_reg, 0, xam->pcie_ecam.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_ecam.base,
+ecam_alias);
+
+/* Map the MMIO space */
+mmio_alias = g_new0(MemoryRegion, 1);
+mmio_reg = sysbus_mmio_get_region(SYS_BUS_DEVICE(dev), 1);
+memory_region_init_alias(mmio_alias, OBJECT(dev), "pcie-mmio",
+ mmio_reg,
+ xam->pcie_mmio.base,
+ xam->pcie_mmio.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_mmio.base,
+mmio_alias);
+
+/* Map the MMIO_HIGH space */
+mmio_alias_high = g_new0(MemoryRegion, 1);
+memory_region_init_alias(mmio_alias_high, OBJECT(dev), "pcie-mmio-high",
+ mmio_reg,
+ xam->pcie_mmio_high.base,
+ xam->pcie_mmio_high.size);
+memory_region_add_subregion(get_system_memory(), xam->pcie_mmio_high.base,
+mmio_alias_high);
+
+/* Legacy PCI interrupts (#INTA - #INTD) */
+for (i = 0; i < GPEX_NUM_IRQS; i++) {
+qemu_irq irq = qemu_allocate_irq(xen_set_irq, NULL,
+ xam->pcie_irq + i);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq);
+gpex_set_irq_num(GPEX_HOST(dev), i, xam->pcie_irq + i);
+}
+
+DPRINTF("Created PCIe host bridge\n");
+}
+
+static bool xen_read_pcie_prop(XenArmState *xam, unsigned int xen_domid,
+   const char *prop_name, unsigned long *data)
+{
+char path[128], *value = NULL;
+unsigned int len;
+bool ret = true;
+
+snprintf(path, sizeof(path), "device-model/%d/pcie_host/%s",
+ xen_domid, prop_name);
+value = 

[PATCH v2 1/6] hw/xen: Set XenBackendInstance in the XenDevice before realizing it

[Volodymyr Babchuk]

From: David Woodhouse 

This allows a XenDevice implementation to know whether it was created
by QEMU, or merely discovered in XenStore after the toolstack created
it. This will allow us to create frontend/backend nodes only when we
should, rather than unconditionally attempting to overwrite them from
a driver domain which doesn't have privileges to do so.

As an added benefit, it also means we no longer have to call the
xen_backend_set_device() function from the device models immediately
after calling qdev_realize_and_unref(). Even though we could make
the argument that it's safe to do so, and the pointer to the unreffed
device *will* actually still be valid, it still made my skin itch to
look at it.

Signed-off-by: David Woodhouse 
---
 hw/block/xen-block.c | 3 +--
 hw/char/xen_console.c| 2 +-
 hw/net/xen_nic.c | 2 +-
 hw/xen/xen-bus.c | 4 
 include/hw/xen/xen-backend.h | 2 --
 include/hw/xen/xen-bus.h | 2 ++
 6 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 6d64ede94f..c2ac9db4a2 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -1081,13 +1081,12 @@ static void xen_block_device_create(XenBackendInstance 
*backend,
 
 blockdev->iothread = iothread;
 blockdev->drive = drive;
+xendev->backend = backend;
 
 if (!qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
 error_prepend(errp, "realization of device %s failed: ", type);
 goto fail;
 }
-
-xen_backend_set_device(backend, xendev);
 return;
 
 fail:
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 5cbee2f184..bef8a3a621 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -600,8 +600,8 @@ static void xen_console_device_create(XenBackendInstance 
*backend,
 goto fail;
 }
 
+xendev->backend = backend;
 if (qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
-xen_backend_set_device(backend, xendev);
 goto done;
 }
 
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index af4ba3f1e6..afa10c96e8 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -627,8 +627,8 @@ static void xen_net_device_create(XenBackendInstance 
*backend,
 net->dev = number;
 memcpy(>conf.macaddr, , sizeof(mac));
 
+xendev->backend = backend;
 if (qdev_realize_and_unref(DEVICE(xendev), BUS(xenbus), errp)) {
-xen_backend_set_device(backend, xendev);
 return;
 }
 
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 4973e7d9c9..dd0171ab98 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -1079,6 +1079,10 @@ static void xen_device_realize(DeviceState *dev, Error 
**errp)
 }
 }
 
+if (xendev->backend) {
+xen_backend_set_device(xendev->backend, xendev);
+}
+
 xendev->exit.notify = xen_device_exit;
 qemu_add_exit_notifier(>exit);
 return;
diff --git a/include/hw/xen/xen-backend.h b/include/hw/xen/xen-backend.h
index 0f01631ae7..ea080ba7c9 100644
--- a/include/hw/xen/xen-backend.h
+++ b/include/hw/xen/xen-backend.h
@@ -10,8 +10,6 @@
 
 #include "hw/xen/xen-bus.h"
 
-typedef struct XenBackendInstance XenBackendInstance;
-
 typedef void (*XenBackendDeviceCreate)(XenBackendInstance *backend,
QDict *opts, Error **errp);
 typedef void (*XenBackendDeviceDestroy)(XenBackendInstance *backend,
diff --git a/include/hw/xen/xen-bus.h b/include/hw/xen/xen-bus.h
index 334ddd1ff6..7647c4c38e 100644
--- a/include/hw/xen/xen-bus.h
+++ b/include/hw/xen/xen-bus.h
@@ -14,9 +14,11 @@
 #include "qom/object.h"
 
 typedef struct XenEventChannel XenEventChannel;
+typedef struct XenBackendInstance XenBackendInstance;
 
 struct XenDevice {
 DeviceState qdev;
+XenBackendInstance *backend;
 domid_t frontend_id;
 char *name;
 struct qemu_xs_handle *xsh;
-- 
2.42.0

[PATCH v2 4/6] xen_pvdev: Do not assume Dom0 when creating a directory

[Volodymyr Babchuk]

From: Oleksandr Tyshchenko 

Instead of forcing the owner to domid 0, use XS_PRESERVE_OWNER to
inherit the owner of the directory.

Note that for other than Dom0 domain (non toolstack domain) the
"driver_domain" property should be set in domain config file for the
toolstack to create required directories in advance.

Signed-off-by: Oleksandr Tyshchenko 
Signed-off-by: Volodymyr Babchuk 
---
 hw/xen/xen_pvdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c
index c5ad71e8dc..42bdd4f6c8 100644
--- a/hw/xen/xen_pvdev.c
+++ b/hw/xen/xen_pvdev.c
@@ -60,7 +60,8 @@ void xen_config_cleanup(void)
 
 int xenstore_mkdir(char *path, int p)
 {
-if (!qemu_xen_xs_create(xenstore, 0, 0, xen_domid, p, path)) {
+if (!qemu_xen_xs_create(xenstore, 0, XS_PRESERVE_OWNER,
+xen_domid, p, path)) {
 xen_pv_printf(NULL, 0, "xs_mkdir %s: failed\n", path);
 return -1;
 }
-- 
2.42.0

[PATCH v2 5/6] xen_arm: Set mc->max_cpus to GUEST_MAX_VCPUS in xen_arm_init()

[Volodymyr Babchuk]

From: Oleksandr Tyshchenko 

The number of vCPUs used for the IOREQ configuration (machine->smp.cpus)
should really match the system value as for each vCPU we setup a dedicated
evtchn for the communication with Xen at the runtime. This is needed
for the IOREQ to be properly configured and work if the involved domain
has more than one vCPU assigned.

Set the number of current supported guest vCPUs here (128) which is
defined in public header arch-arm.h. And the toolstack should then
pass max_vcpus using "-smp" arg.

Signed-off-by: Oleksandr Tyshchenko 
Signed-off-by: Volodymyr Babchuk 
---
 hw/arm/xen_arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index a5631529d0..b9c3ae14b6 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -231,7 +231,7 @@ static void xen_arm_machine_class_init(ObjectClass *oc, 
void *data)
 MachineClass *mc = MACHINE_CLASS(oc);
 mc->desc = "Xen Para-virtualized PC";
 mc->init = xen_arm_init;
-mc->max_cpus = 1;
+mc->max_cpus = GUEST_MAX_VCPUS;
 mc->default_machine_opts = "accel=xen";
 /* Set explicitly here to make sure that real ram_size is passed */
 mc->default_ram_size = 0;
-- 
2.42.0

[PATCH v2 0/6] xen-arm: add support for virtio-pci

[Volodymyr Babchuk]

Hello,

This patch series adds the basic support for virtio-pci for xen-arm
guests. The main changes are in "xen_arm: Add virtual PCIe host bridge
support", while most of other patches are required to make QEMU work
as device model in a non-privileged domains like driver domain.

Changes v1->v2:

 - Removed "xen-bus: Set offline if backend's state is XenbusStateClosed"

 - Included David's patch as it is the pre-req for "xen: backends:
   touch some XenStore nodes only if device..."

 ---

David Woodhouse (1):
  hw/xen: Set XenBackendInstance in the XenDevice before realizing it

Oleksandr Tyshchenko (3):
  xen_pvdev: Do not assume Dom0 when creating a directory
  xen_arm: Set mc->max_cpus to GUEST_MAX_VCPUS in xen_arm_init()
  xen_arm: Add virtual PCIe host bridge support

Volodymyr Babchuk (2):
  xen: backends: touch some XenStore nodes only if device...
  xen: xenstore: add possibility to preserve owner

 hw/arm/xen_arm.c | 188 ++-
 hw/block/xen-block.c |  19 ++--
 hw/char/xen_console.c|   4 +-
 hw/i386/kvm/xen_xenstore.c   |  18 +++
 hw/net/xen_nic.c |  20 ++--
 hw/xen/xen-bus.c |  18 ++-
 hw/xen/xen-hvm-common.c  |   9 +-
 hw/xen/xen-operations.c  |  12 ++
 hw/xen/xen_pvdev.c   |   3 +-
 include/hw/xen/xen-backend.h |   2 -
 include/hw/xen/xen-bus.h |   2 +
 include/hw/xen/xen_backend_ops.h |   7 ++
 include/hw/xen/xen_native.h  |   8 +-
 13 files changed, 278 insertions(+), 32 deletions(-)

-- 
2.42.0

[PATCH v2 3/6] xen: xenstore: add possibility to preserve owner

[Volodymyr Babchuk]

Add option to preserve owner when creating an entry in Xen Store. This
may be needed in cases when Qemu is working as device model in a
domain that is Domain-0, e.g. in driver domain.

"owner" parameter for qemu_xen_xs_create() function can have special
value XS_PRESERVE_OWNER, which will make specific implementation to
get original owner of an entry and pass it back to
set_permissions() call.

Please note, that XenStore inherits permissions, so even if entry is
newly created by, it already has the owner set to match owner of entry
at previous level.

Signed-off-by: Volodymyr Babchuk 

--

In v2:

 - Pass transaction to xs_get_permissions() in libxenstore_create()
 - Added comment before XS_PRESERVE_OWNER defintion
 - Extended the commit message
---
 hw/i386/kvm/xen_xenstore.c   | 18 ++
 hw/xen/xen-operations.c  | 12 
 include/hw/xen/xen_backend_ops.h |  7 +++
 3 files changed, 37 insertions(+)

diff --git a/hw/i386/kvm/xen_xenstore.c b/hw/i386/kvm/xen_xenstore.c
index 6e651960b3..d0fd5d4681 100644
--- a/hw/i386/kvm/xen_xenstore.c
+++ b/hw/i386/kvm/xen_xenstore.c
@@ -1595,6 +1595,24 @@ static bool xs_be_create(struct qemu_xs_handle *h, 
xs_transaction_t t,
 return false;
 }
 
+if (owner == XS_PRESERVE_OWNER) {
+GList *prev_perms;
+char letter;
+
+err = xs_impl_get_perms(h->impl, 0, t, path, _perms);
+if (err) {
+errno = err;
+return false;
+}
+
+if (sscanf(prev_perms->data, "%c%u", , ) != 2) {
+errno = EFAULT;
+g_list_free_full(prev_perms, g_free);
+return false;
+}
+g_list_free_full(prev_perms, g_free);
+}
+
 perms_list = g_list_append(perms_list,
xs_perm_as_string(XS_PERM_NONE, owner));
 perms_list = g_list_append(perms_list,
diff --git a/hw/xen/xen-operations.c b/hw/xen/xen-operations.c
index e00983ec44..ae8265635f 100644
--- a/hw/xen/xen-operations.c
+++ b/hw/xen/xen-operations.c
@@ -300,6 +300,18 @@ static bool libxenstore_create(struct qemu_xs_handle *h, 
xs_transaction_t t,
 return false;
 }
 
+if (owner == XS_PRESERVE_OWNER) {
+struct xs_permissions *tmp;
+unsigned int num;
+
+tmp = xs_get_permissions(h->xsh, t, path, );
+if (tmp == NULL) {
+return false;
+}
+perms_list[0].id = tmp[0].id;
+free(tmp);
+}
+
 return xs_set_permissions(h->xsh, t, path, perms_list,
   ARRAY_SIZE(perms_list));
 }
diff --git a/include/hw/xen/xen_backend_ops.h b/include/hw/xen/xen_backend_ops.h
index 90cca85f52..79021538a3 100644
--- a/include/hw/xen/xen_backend_ops.h
+++ b/include/hw/xen/xen_backend_ops.h
@@ -266,6 +266,13 @@ typedef uint32_t xs_transaction_t;
 #define XS_PERM_READ  0x01
 #define XS_PERM_WRITE 0x02
 
+/*
+ * This is QEMU-specific special value used only by QEMU wrappers
+ * around XenStore. It can be passed to qemu_xen_xs_create() to
+ * inherit owner value from higher-level XS entry.
+ */
+#define XS_PRESERVE_OWNER0xFFFE
+
 struct xenstore_backend_ops {
 struct qemu_xs_handle *(*open)(void);
 void (*close)(struct qemu_xs_handle *h);
-- 
2.42.0

[PATCH v2 2/6] xen: backends: touch some XenStore nodes only if device...

[Volodymyr Babchuk]

was created by QEMU

Xen PV devices in QEMU can be created in two ways: either by QEMU
itself, if they were passed via command line, or by Xen toolstack. In
the latter case, QEMU scans XenStore entries and configures devices
accordingly.

In the second case we don't want QEMU to write/delete front-end
entries for two reasons: it might have no access to those entries if
it is running in un-privileged domain and it is just incorrect to
overwrite entries already provided by Xen toolstack, because toolstack
manages those nodes. For example, it might read backend- or frontend-
state to be sure that they are both disconnected and it is safe to
destroy a domain.

This patch checks presence of xendev->backend to check if Xen PV
device is acting as a backend (i.e. it was configured by Xen
toolstack) to decide if it should touch frontend entries in XenStore.
Also, when we need to remove XenStore entries during device teardown
only if they weren't created by Xen toolstack. If they were created by
toolstack, then it is toolstack's job to do proper clean-up.

Suggested-by: Paul Durrant 
Suggested-by: David Woodhouse 
Co-Authored-by: Oleksandr Tyshchenko 
Signed-off-by: Volodymyr Babchuk 
---
 hw/block/xen-block.c  | 16 +---
 hw/char/xen_console.c |  2 +-
 hw/net/xen_nic.c  | 18 ++
 hw/xen/xen-bus.c  | 14 +-
 4 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index c2ac9db4a2..dac519a6d3 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -390,13 +390,15 @@ static void xen_block_realize(XenDevice *xendev, Error 
**errp)
 
 xen_device_backend_printf(xendev, "info", "%u", blockdev->info);
 
-xen_device_frontend_printf(xendev, "virtual-device", "%lu",
-   vdev->number);
-xen_device_frontend_printf(xendev, "device-type", "%s",
-   blockdev->device_type);
-
-xen_device_backend_printf(xendev, "sector-size", "%u",
-  conf->logical_block_size);
+if (!xendev->backend) {
+xen_device_frontend_printf(xendev, "virtual-device", "%lu",
+   vdev->number);
+xen_device_frontend_printf(xendev, "device-type", "%s",
+   blockdev->device_type);
+
+xen_device_backend_printf(xendev, "sector-size", "%u",
+  conf->logical_block_size);
+}
 
 xen_block_set_size(blockdev);
 
diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index bef8a3a621..b52abf 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -450,7 +450,7 @@ static void xen_console_realize(XenDevice *xendev, Error 
**errp)
 
 trace_xen_console_realize(con->dev, object_get_typename(OBJECT(cs)));
 
-if (CHARDEV_IS_PTY(cs)) {
+if (CHARDEV_IS_PTY(cs) && !xendev->backend) {
 /* Strip the leading 'pty:' */
 xen_device_frontend_printf(xendev, "tty", "%s", cs->filename + 4);
 }
diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index afa10c96e8..27442bef38 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -315,14 +315,16 @@ static void xen_netdev_realize(XenDevice *xendev, Error 
**errp)
 
 qemu_macaddr_default_if_unset(>conf.macaddr);
 
-xen_device_frontend_printf(xendev, "mac", "%02x:%02x:%02x:%02x:%02x:%02x",
-   netdev->conf.macaddr.a[0],
-   netdev->conf.macaddr.a[1],
-   netdev->conf.macaddr.a[2],
-   netdev->conf.macaddr.a[3],
-   netdev->conf.macaddr.a[4],
-   netdev->conf.macaddr.a[5]);
-
+if (!xendev->backend) {
+xen_device_frontend_printf(xendev, "mac",
+   "%02x:%02x:%02x:%02x:%02x:%02x",
+   netdev->conf.macaddr.a[0],
+   netdev->conf.macaddr.a[1],
+   netdev->conf.macaddr.a[2],
+   netdev->conf.macaddr.a[3],
+   netdev->conf.macaddr.a[4],
+   netdev->conf.macaddr.a[5]);
+}
 netdev->nic = qemu_new_nic(_xen_info, >conf,
object_get_typename(OBJECT(xendev)),
DEVICE(xendev)->id, netdev);
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index dd0171ab98..d0f17aeb27 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -599,8 +599,10 @@ static void xen_device_backend_destroy(XenDevice *xendev)
 
 g_assert(xenbus->xsh);
 
-xs_node_destroy(xenbus->xsh, XBT_NULL, xendev->backend_path,
-_err);
+if (!xendev->backend) {
+xs_node_destroy(xenbus->xsh, XBT_NULL, xendev->backend_path,
+_err);
+}
 g_free(xendev->backend_path);
 

Re: [PATCH V5 02/12] cpus: stop vm in suspended state

[Steven Sistare]

On 11/20/2023 4:44 PM, Peter Xu wrote:
> On Mon, Nov 20, 2023 at 03:55:54PM -0500, Steven Sistare wrote:
>> If we drop force, then all calls to vm_stop will completely stop the
>> suspended state, eg an hmp "stop" command. This causes two problems.
>> First, that is a change in user-visible behavior for something that
>> currently works,
> 
> IMHO it depends on what should be the correct behavior.  IOW, when VM is in
> SUSPENDED state and then the user sends "stop" QMP command, what should we
> expect?
> 
> My understanding is we should expect to fully stop the VM, including the
> ticks, for example.  Keeping the ticks running even after QMP "stop"
> doesn't sound right, isn't it?

I agree, but others may not, and this decision would require approval from 
maintainers in other areas, including upper layers such as libvirt that are
aware of the suspended state.  It is a user-visible change, and may require 
a new libvirt release.

>> vs the migration code where we are fixing brokenness.
> 
> This is not a migration-only bug if above holds, IMO.
> 
>> Second, it does not quite work, because the state becomes
>> RUN_STATE_PAUSED, so the suspended state is forgotten, and the hmp "cont"
>> will try to set the running state.  I could fix that by introducing a new
>> state RUN_STATE_SUSPENDED_STOPPED, but again it is a user-visible change
>> in existing behavior.  (I even implemented that while developing, then I
>> realized it was not needed to fix the migration bugs.)
> 
> Good point.
> 
> Now with above comments, what's your thoughts on whether we should change
> the user behavior?  My answer is still a yes.
> 
> Maybe SUSPENDED should not be a RunState at all? SUSPENDED is guest visible
> behavior, while something like QMP "stop" is not guest visible.  Maybe we
> should remember it separately?
>
> It means qemu_system_suspend() could remember that in a separate field (as
> part of guest state), then when wakeup we should conditionally go back
> with/without vcpus running depending on the new "suspended" state.

Regardless of how we remember it, the status command must still expose the 
suspended state to the user.  The user must be able to see that a guest is 
suspended, and decide when to issue a wakeup command.

If we change the stop command to completely stop a suspended vm, then we must
allow the user to query whether a vm is suspended-running or suspended-stopped,
because the command they must issue to resume is different: wakeup for the
former, and cont for the latter.

This change is visible to libvirt.  Adding it will delay this entire patch
series, and is not necessary for fixing the migration bug.  There is no
downside to drawing the line here for this series, and possibly changing stop
semantics in the future.

- Steve

Re: [PATCH qemu 1/1] Implement STM32L4x5 EXTI

[Philippe Mathieu-Daudé]

Hi Arnaud,

On 11/11/23 15:33, ~aminier wrote:

From: Arnaud Minier 

Signed-off-by: Arnaud Minier 
Signed-off-by: Inès Varhol 
---
  hw/arm/Kconfig|   1 +
  hw/arm/stm32l4x5_soc.c|  65 +-
  hw/misc/Kconfig   |   3 +
  hw/misc/meson.build   |   1 +
  hw/misc/stm32l4x5_exti.c  | 329 ++
  hw/misc/trace-events  |   5 +
  include/hw/arm/stm32l4x5_soc.h|   3 +
  include/hw/misc/stm32l4x5_exti.h  |  64 ++
  tests/qtest/meson.build   |   5 +
  tests/qtest/stm32l4x5_exti-test.c | 102 +
  10 files changed, 576 insertions(+), 2 deletions(-)
  create mode 100644 hw/misc/stm32l4x5_exti.c
  create mode 100644 include/hw/misc/stm32l4x5_exti.h
  create mode 100644 tests/qtest/stm32l4x5_exti-test.c




diff --git a/hw/arm/stm32l4x5_soc.c b/hw/arm/stm32l4x5_soc.c
index 198d3f6d3e..6f2a1b34b3 100644
--- a/hw/arm/stm32l4x5_soc.c
+++ b/hw/arm/stm32l4x5_soc.c
@@ -43,10 +43,51 @@
  #define SRAM2_BASE_ADDRESS 0x1000
  #define SRAM2_SIZE (32 * KiB)
  
+static const hwaddr exti_addr = 0x40010400;


Why not a #define?


+#define NUM_EXTI_IRQ 40




diff --git a/hw/misc/stm32l4x5_exti.c b/hw/misc/stm32l4x5_exti.c




+static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
+{
+Stm32l4x5ExtiState *s = opaque;
+
+trace_stm32l4x5_exti_set_irq(irq, level);
+
+if (irq >= NUM_INTERRUPT_OUT_LINES) {
+return;


This can not happen. If you are unsure, this would be a programming
error, thus aborting is better, but nothing needed IMO.


+}
+
+if (irq < 32) {
+if (((1 << irq) & s->exti_rtsr1) && level) {
+/* Rising Edge */
+s->exti_pr1 |= 1 << irq;
+}
+
+if (((1 << irq) & s->exti_ftsr1) && !level) {
+/* Falling Edge */
+s->exti_pr1 |= 1 << irq;
+}
+
+if (!((1 << irq) & s->exti_imr1)) {
+/* Interrupt is masked */
+return;
+}
+} else {
+/* Shift the value to enable access in x2 registers*/
+int irq_x2 = irq - 32;
+if (((1 << irq_x2) & s->exti_rtsr2) && level) {
+/* Rising Edge */
+s->exti_pr2 |= 1 << irq_x2;
+}
+
+if (((1 << irq_x2) & s->exti_ftsr2) && !level) {
+/* Falling Edge */
+s->exti_pr2 |= 1 << irq_x2;
+}
+
+if (!((1 << irq_x2) & s->exti_imr2)) {
+/* Interrupt is masked */
+return;
+}
+}
+qemu_irq_pulse(s->irq[irq]);
+}


Could be simpler avoiding duplication, as:

---
static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
{
Stm32l4x5ExtiState *s = opaque;
int oirq = irq;
uint32_t *rtsr;
uint32_t *ftsr;
uint32_t *pr;
uint32_t *imr;

trace_stm32l4x5_exti_set_irq(irq, level);

if (irq < 32) {
rtsr = >exti_rtsr1;
ftsr = >exti_ftsr1;
pr = >exti_pr1;
imr = >exti_imr1;
} else {
rtsr = >exti_rtsr2;
ftsr = >exti_ftsr2;
pr = >exti_pr2;
imr = >exti_imr2;
/* Shift the value to enable access in x2 registers. */
irq -= 32;
}

if (((1 << irq) & *rtsr) && level) {
/* Rising Edge */
*pr |= 1 << irq;
}

if (((1 << irq) & *ftsr) && !level) {
/* Falling Edge */
*pr |= 1 << irq;
}

if (!((1 << irq) & *imr)) {
/* Interrupt is masked */
return;
}

qemu_irq_pulse(s->irq[oirq]);
}
---

But changing Stm32l4x5ExtiState as:

---
struct Stm32l4x5ExtiState {
SysBusDevice parent_obj;

MemoryRegion mmio;

uint32_t imr[2];
uint32_t emr[2];
uint32_t rtsr[2];
uint32_t ftsr[2];
uint32_t swier[2];
uint32_t pr[2];

qemu_irq irq[NUM_INTERRUPT_OUT_LINES];
};
---

We get even simpler:

---
static void stm32l4x5_exti_set_irq(void *opaque, int irq, int level)
{
Stm32l4x5ExtiState *s = opaque;
unsigned n = irq >= 32;
int oirq = irq;

trace_stm32l4x5_exti_set_irq(irq, level);

if (irq >= 32) {
/* Shift the value to enable access in x2 registers. */
irq -= 32;
}

if (((1 << irq) & s->rtsr[n]) && level) {
/* Rising Edge */
s->pr[n] |= 1 << irq;
}

if (((1 << irq) & s->ftsr[n]) && !level) {
/* Falling Edge */
s->pr[n] |= 1 << irq;
}

if (!((1 << irq) & s->imr[n])) {
/* Interrupt is masked */
return;
}

qemu_irq_pulse(s->irq[oirq]);
}
---

(code untested).


+
+static uint64_t stm32l4x5_exti_read(void *opaque, hwaddr addr,
+unsigned int size)
+{
+Stm32l4x5ExtiState *s = opaque;
+uint32_t r = 0;


   unsigned n = addr >= EXTI_IMR2;

+
+switch (addr) {
+case EXTI_IMR1:
+r = s->exti_imr1;
+break;


This becomes:

   case EXTI_IMR1:
   case EXTI_IMR2:
   r = s->exti_imr[n];
   break;
   ...

[PATCH 0/1 for qemu-8.2] SeaBIOS-hppa v13

[deller]

From: Helge Deller 

Please pull an update of SeaBIOS-hppa to v13 to fix
a system reboot crash in qemu-system-hppa as reported in
https://gitlab.com/qemu-project/qemu/-/issues/1991

Helge Deller (1):
  target/hppa: Update SeaBIOS-hppa to version 13

 pc-bios/hppa-firmware.img | Bin 681332 -> 681388 bytes
 roms/seabios-hppa |   2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)

-- 
2.41.0

[PATCH-for-9.0] meson: Allow building binary with no target-specific files in hw/

[Philippe Mathieu-Daudé]

Allow  building a qemu-system-foo binary with target-agnostic
only HW models.

Signed-off-by: Philippe Mathieu-Daudé 
---
Although this is for 9.0, I wanted to post it today to celebrate :)

Once reviewed I plan to queue it via my heterogenenous-cpus tree.
---
 meson.build | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index ec01f8b138..9ccfb091e7 100644
--- a/meson.build
+++ b/meson.build
@@ -3772,9 +3772,11 @@ foreach target : target_dirs
 arch_deps += t.dependencies()
 
 hw_dir = target_name == 'sparc64' ? 'sparc64' : target_base_arch
-hw = hw_arch[hw_dir].apply(config_target, strict: false)
-arch_srcs += hw.sources()
-arch_deps += hw.dependencies()
+if hw_arch.has_key(hw_dir)
+  hw = hw_arch[hw_dir].apply(config_target, strict: false)
+  arch_srcs += hw.sources()
+  arch_deps += hw.dependencies()
+endif
 
 arch_srcs += config_devices_h[target]
 link_args += ['@block.syms', '@qemu.syms']
-- 
2.41.0

Re: [PATCH v4 05/11] ppc/pnv: Wire up pca9552 GPIO pins for PCIe hotplug power control

[Miles Glenn]

On Tue, 2023-11-21 at 19:36 +0100, Cédric Le Goater wrote:
> On 11/21/23 00:51, Glenn Miles wrote:
> > For power10-rainier, a pca9552 device is used for PCIe slot hotplug
> > power control by the Power Hypervisor code.  The code expects that
> > some time after it enables power to a PCIe slot by asserting one of
> > the pca9552 GPIO pins 0-4, it should see a "power good" signal
> > asserted
> > on one of pca9552 GPIO pins 5-9.
> 
> And this is what OPAL is not doing :
> 
>
> https://github.com/open-power/skiboot/blob/master/platforms/astbmc/rainier.c#L65
> 
> Correct ?
> 

Ah, yes, I believe you are correct!

> > To simulate this behavior, we simply connect the GPIO outputs for
> > pins 0-4 to the GPIO inputs for pins 5-9.
> > 
> > Each PCIe slot is assigned 3 GPIO pins on the pca9552 device, for
> > control of up to 5 PCIe slots.  The per-slot signal names are:
> > 
> > SLOTx_EN...PHYP uses this as an output to enable
> >slot power.  We connect this to the
> >SLOTx_PG pin to simulate a PGOOD signal.
> > SLOTx_PG...PHYP uses this as in input to detect
> >PGOOD for the slot.  For our purposes
> >we just connect this to the SLOTx_EN
> >output.
> > SLOTx_Control..PHYP uses this as an output to prevent
> >a race condition in the real hotplug
> >circuitry, but we can ignore this output
> >for simulation.
> > 
> > Signed-off-by: Glenn Miles 
> > ---
> > 
> > Changes from previous version:
> >- Code moved from pnv_chip_power10_realize to
> > pnv_rainier_i2c_init
> > 
> >   hw/ppc/pnv.c | 20 ++--
> >   1 file changed, 18 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 9cefcd0fd6..80d25fc1bd 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -1898,8 +1898,24 @@ static void
> > pnv_rainier_i2c_init(PnvMachineState *pnv)
> >* Add a PCA9552 I2C device for PCIe hotplug control
> >* to engine 2, bus 1, address 0x63
> >*/
> > -i2c_slave_create_simple(chip10->i2c[2].busses[1],
> > -"pca9552", 0x63);
> > +I2CSlave *hotplug = i2c_slave_create_simple(chip10-
> > >i2c[2].busses[1],
> > +"pca9552", 0x63);
> 
> hotplug ? why not dev simply ?
> 
> 
> Thanks,
> 
> C.
> 
> 
Sure, dev is fine.  I'll change it.

Thanks,

Glenn

> 
> > +
> > +/*
> > + * Connect PCA9552 GPIO pins 0-4 (SLOTx_EN) outputs to
> > GPIO pins 5-9
> > + * (SLOTx_PG) inputs in order to fake the pgood state of
> > PCIe slots
> > + * after hypervisor code sets a SLOTx_EN pin high.
> > + */
> > +qdev_connect_gpio_out(DEVICE(hotplug), 0,
> > +  qdev_get_gpio_in(DEVICE(hotplug),
> > 5));
> > +qdev_connect_gpio_out(DEVICE(hotplug), 1,
> > +  qdev_get_gpio_in(DEVICE(hotplug),
> > 6));
> > +qdev_connect_gpio_out(DEVICE(hotplug), 2,
> > +  qdev_get_gpio_in(DEVICE(hotplug),
> > 7));
> > +qdev_connect_gpio_out(DEVICE(hotplug), 3,
> > +  qdev_get_gpio_in(DEVICE(hotplug),
> > 8));
> > +qdev_connect_gpio_out(DEVICE(hotplug), 4,
> > +  qdev_get_gpio_in(DEVICE(hotplug),
> > 9));
> >   }
> >   }
> >   

[PATCH v5 5/9] ppc/pnv: Wire up pca9552 GPIO pins for PCIe hotplug power control

[Glenn Miles]

For power10-rainier, a pca9552 device is used for PCIe slot hotplug
power control by the Power Hypervisor code.  The code expects that
some time after it enables power to a PCIe slot by asserting one of
the pca9552 GPIO pins 0-4, it should see a "power good" signal asserted
on one of pca9552 GPIO pins 5-9.

To simulate this behavior, we simply connect the GPIO outputs for
pins 0-4 to the GPIO inputs for pins 5-9.

Each PCIe slot is assigned 3 GPIO pins on the pca9552 device, for
control of up to 5 PCIe slots.  The per-slot signal names are:

   SLOTx_EN...PHYP uses this as an output to enable
  slot power.  We connect this to the
  SLOTx_PG pin to simulate a PGOOD signal.
   SLOTx_PG...PHYP uses this as in input to detect
  PGOOD for the slot.  For our purposes
  we just connect this to the SLOTx_EN
  output.
   SLOTx_Control..PHYP uses this as an output to prevent
  a race condition in the real hotplug
  circuitry, but we can ignore this output
  for simulation.

Signed-off-by: Glenn Miles 
---

No change from previous version

 hw/ppc/pnv.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index d8d19fb065..088824fd9f 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1900,7 +1900,24 @@ static void pnv_rainier_i2c_init(PnvMachineState *pnv)
  * Add a PCA9552 I2C device for PCIe hotplug control
  * to engine 2, bus 1, address 0x63
  */
-i2c_slave_create_simple(chip10->i2c[2].busses[1], "pca9552", 0x63);
+I2CSlave *hotplug = i2c_slave_create_simple(chip10->i2c[2].busses[1],
+"pca9552", 0x63);
+
+/*
+ * Connect PCA9552 GPIO pins 0-4 (SLOTx_EN) outputs to GPIO pins 5-9
+ * (SLOTx_PG) inputs in order to fake the pgood state of PCIe slots
+ * after hypervisor code sets a SLOTx_EN pin high.
+ */
+qdev_connect_gpio_out(DEVICE(hotplug), 0,
+  qdev_get_gpio_in(DEVICE(hotplug), 5));
+qdev_connect_gpio_out(DEVICE(hotplug), 1,
+  qdev_get_gpio_in(DEVICE(hotplug), 6));
+qdev_connect_gpio_out(DEVICE(hotplug), 2,
+  qdev_get_gpio_in(DEVICE(hotplug), 7));
+qdev_connect_gpio_out(DEVICE(hotplug), 3,
+  qdev_get_gpio_in(DEVICE(hotplug), 8));
+qdev_connect_gpio_out(DEVICE(hotplug), 4,
+  qdev_get_gpio_in(DEVICE(hotplug), 9));
 }
 }
 
-- 
2.31.1

[PATCH v5 9/9] ppc/pnv: Test pnv i2c master and connected devices

[Glenn Miles]

Tests the following for both P9 and P10:
  - I2C master POR status
  - I2C master status after immediate reset

Tests the following for powernv10-ranier only:
  - Config pca9552 hotplug device pins as inputs then
Read the INPUT0/1 registers to verify all pins are high
  - Connected GPIO pin tests of P10 PCA9552 device.  Tests
output of pins 0-4 affect input of pins 5-9 respectively.
  - PCA9554 GPIO pins test.  Tests input and ouput functionality.

Signed-off-by: Glenn Miles 
---

No change from previous version

 tests/qtest/meson.build |   1 +
 tests/qtest/pnv-host-i2c-test.c | 650 
 2 files changed, 651 insertions(+)
 create mode 100644 tests/qtest/pnv-host-i2c-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 47dabf91d0..fbb0bd204c 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -163,6 +163,7 @@ qtests_ppc64 = \
   qtests_ppc + \
   (config_all_devices.has_key('CONFIG_PSERIES') ? ['device-plug-test'] : []) + 
  \
   (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-xscom-test'] : []) +   
  \
+  (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-host-i2c-test'] : []) 
+  \
   (config_all_devices.has_key('CONFIG_PSERIES') ? ['rtas-test'] : []) +
  \
   (slirp.found() ? ['pxe-test'] : []) +  \
   (config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] : []) 
+ \
diff --git a/tests/qtest/pnv-host-i2c-test.c b/tests/qtest/pnv-host-i2c-test.c
new file mode 100644
index 00..377525e458
--- /dev/null
+++ b/tests/qtest/pnv-host-i2c-test.c
@@ -0,0 +1,650 @@
+/*
+ * QTest testcase for PowerNV 10 Host I2C Communications
+ *
+ * Copyright (c) 2023, IBM Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later. See the COPYING file in the top-level directory.
+ */
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "hw/misc/pca9554_regs.h"
+#include "hw/misc/pca9552_regs.h"
+
+#define PPC_BIT(bit)(0x8000ULL >> (bit))
+#define PPC_BIT32(bit)  (0x8000 >> (bit))
+#define PPC_BIT8(bit)   (0x80 >> (bit))
+#define PPC_BITMASK(bs, be) ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
+#define PPC_BITMASK32(bs, be)   ((PPC_BIT32(bs) - PPC_BIT32(be)) | \
+ PPC_BIT32(bs))
+
+#define MASK_TO_LSH(m)  (__builtin_ffsll(m) - 1)
+#define GETFIELD(m, v)  (((v) & (m)) >> MASK_TO_LSH(m))
+#define SETFIELD(m, v, val) \
+(((v) & ~(m)) | typeof(v))(val)) << MASK_TO_LSH(m)) & (m)))
+
+#define P10_XSCOM_BASE  0x000603fcull
+#define PNV10_CHIP_MAX_I2C  5
+#define PNV10_XSCOM_I2CM_BASE   0xa
+#define PNV10_XSCOM_I2CM_SIZE   0x1000
+
+/* I2C FIFO register */
+#define I2C_FIFO_REG0x4
+#define I2C_FIFOPPC_BITMASK(0, 7)
+
+/* I2C command register */
+#define I2C_CMD_REG 0x5
+#define I2C_CMD_WITH_START  PPC_BIT(0)
+#define I2C_CMD_WITH_ADDR   PPC_BIT(1)
+#define I2C_CMD_READ_CONT   PPC_BIT(2)
+#define I2C_CMD_WITH_STOP   PPC_BIT(3)
+#define I2C_CMD_INTR_STEERING   PPC_BITMASK(6, 7) /* P9 */
+#define   I2C_CMD_INTR_STEER_HOST   1
+#define   I2C_CMD_INTR_STEER_OCC2
+#define I2C_CMD_DEV_ADDRPPC_BITMASK(8, 14)
+#define I2C_CMD_READ_NOT_WRITE  PPC_BIT(15)
+#define I2C_CMD_LEN_BYTES   PPC_BITMASK(16, 31)
+#define I2C_MAX_TFR_LEN 0xfff0ull
+
+/* I2C mode register */
+#define I2C_MODE_REG0x6
+#define I2C_MODE_BIT_RATE_DIV   PPC_BITMASK(0, 15)
+#define I2C_MODE_PORT_NUM   PPC_BITMASK(16, 21)
+#define I2C_MODE_ENHANCED   PPC_BIT(28)
+#define I2C_MODE_DIAGNOSTIC PPC_BIT(29)
+#define I2C_MODE_PACING_ALLOW   PPC_BIT(30)
+#define I2C_MODE_WRAP   PPC_BIT(31)
+
+/* I2C watermark register */
+#define I2C_WATERMARK_REG   0x7
+#define I2C_WATERMARK_HIGH  PPC_BITMASK(16, 19)
+#define I2C_WATERMARK_LOW   PPC_BITMASK(24, 27)
+
+/*
+ * I2C interrupt mask and condition registers
+ *
+ * NB: The function of 0x9 and 0xa changes depending on whether you're reading
+ * or writing to them. When read they return the interrupt condition bits
+ * and on writes they update the interrupt mask register.
+ *
+ *  The bit definitions are the same for all the interrupt registers.
+ */
+#define I2C_INTR_MASK_REG   0x8
+
+#define I2C_INTR_RAW_COND_REG   0x9 /* read */
+#define I2C_INTR_MASK_OR_REG0x9 /* write*/
+
+#define I2C_INTR_COND_REG   0xa /* read */
+#define I2C_INTR_MASK_AND_REG   0xa /* write */
+
+#define I2C_INTR_ALLPPC_BITMASK(16, 31)
+#define I2C_INTR_INVALID_CMDPPC_BIT(16)
+#define I2C_INTR_LBUS_PARITY_ERR  

[PATCH v5 3/9] ppc/pnv: New powernv10-rainier machine type

[Glenn Miles]

Create a new powernv machine type, powernv10-rainier, that
will contain rainier-specific devices.

Signed-off-by: Glenn Miles 
---

Changes from previous version:
  - Formatting changes
  - Capitalized "Rainier" in machine description string
  - Changed powernv10-rainier parent to MACHINE_TYPE_NAME("powernv10")

 hw/ppc/pnv.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0297871bdd..08704ce695 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2251,7 +2251,7 @@ static void pnv_machine_power9_class_init(ObjectClass 
*oc, void *data)
 machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
 }
 
-static void pnv_machine_power10_class_init(ObjectClass *oc, void *data)
+static void pnv_machine_p10_common_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
 PnvMachineClass *pmc = PNV_MACHINE_CLASS(oc);
@@ -2263,7 +2263,6 @@ static void pnv_machine_power10_class_init(ObjectClass 
*oc, void *data)
 { TYPE_PNV_PHB_ROOT_PORT, "version", "5" },
 };
 
-mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";
 mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power10_v2.0");
 compat_props_add(mc->compat_props, phb_compat, G_N_ELEMENTS(phb_compat));
 
@@ -2276,6 +2275,22 @@ static void pnv_machine_power10_class_init(ObjectClass 
*oc, void *data)
 machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
 }
 
+static void pnv_machine_power10_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+pnv_machine_p10_common_class_init(oc, data);
+mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";
+}
+
+static void pnv_machine_p10_rainier_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+pnv_machine_p10_common_class_init(oc, data);
+mc->desc = "IBM PowerNV (Non-Virtualized) POWER10 Rainier";
+}
+
 static bool pnv_machine_get_hb(Object *obj, Error **errp)
 {
 PnvMachineState *pnv = PNV_MACHINE(obj);
@@ -2381,6 +2396,11 @@ static void pnv_machine_class_init(ObjectClass *oc, void 
*data)
 }
 
 static const TypeInfo types[] = {
+{
+.name  = MACHINE_TYPE_NAME("powernv10-rainier"),
+.parent= MACHINE_TYPE_NAME("powernv10"),
+.class_init= pnv_machine_p10_rainier_class_init,
+},
 {
 .name  = MACHINE_TYPE_NAME("powernv10"),
 .parent= TYPE_PNV_MACHINE,
-- 
2.31.1

[PATCH v5 7/9] misc: Add a pca9554 GPIO device model

[Glenn Miles]

Specs are available here:

https://www.nxp.com/docs/en/data-sheet/PCA9554_9554A.pdf

This is a simple model supporting the basic registers for GPIO
mode.  The device also supports an interrupt output line but the
model does not yet support this.

Reviewed-by: Cédric Le Goater 
Signed-off-by: Glenn Miles 
---

No change from previous version

 MAINTAINERS|  10 +-
 hw/misc/pca9554.c  | 328 +
 include/hw/misc/pca9554.h  |  36 
 include/hw/misc/pca9554_regs.h |  19 ++
 4 files changed, 391 insertions(+), 2 deletions(-)
 create mode 100644 hw/misc/pca9554.c
 create mode 100644 include/hw/misc/pca9554.h
 create mode 100644 include/hw/misc/pca9554_regs.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 695e0bd34f..4d1c991691 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1155,9 +1155,7 @@ R: Joel Stanley 
 L: qemu-...@nongnu.org
 S: Maintained
 F: hw/*/*aspeed*
-F: hw/misc/pca9552.c
 F: include/hw/*/*aspeed*
-F: include/hw/misc/pca9552*.h
 F: hw/net/ftgmac100.c
 F: include/hw/net/ftgmac100.h
 F: docs/system/arm/aspeed.rst
@@ -1526,6 +1524,14 @@ F: include/hw/pci-host/pnv*
 F: pc-bios/skiboot.lid
 F: tests/qtest/pnv*
 
+pca955x
+M: Glenn Miles 
+L: qemu-...@nongnu.org
+L: qemu-...@nongnu.org
+S: Odd Fixes
+F: hw/misc/pca955*.c
+F: include/hw/misc/pca955*.h
+
 virtex_ml507
 M: Edgar E. Iglesias 
 L: qemu-...@nongnu.org
diff --git a/hw/misc/pca9554.c b/hw/misc/pca9554.c
new file mode 100644
index 00..778b32e443
--- /dev/null
+++ b/hw/misc/pca9554.c
@@ -0,0 +1,328 @@
+/*
+ * PCA9554 I/O port
+ *
+ * Copyright (c) 2023, IBM Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qemu/bitops.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/pca9554.h"
+#include "hw/misc/pca9554_regs.h"
+#include "hw/irq.h"
+#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "qapi/visitor.h"
+#include "trace.h"
+#include "qom/object.h"
+
+struct PCA9554Class {
+/*< private >*/
+I2CSlaveClass parent_class;
+/*< public >*/
+};
+typedef struct PCA9554Class PCA9554Class;
+
+DECLARE_CLASS_CHECKERS(PCA9554Class, PCA9554,
+   TYPE_PCA9554)
+
+#define PCA9554_PIN_LOW  0x0
+#define PCA9554_PIN_HIZ  0x1
+
+static const char *pin_state[] = {"low", "high"};
+
+static void pca9554_update_pin_input(PCA9554State *s)
+{
+int i;
+uint8_t config = s->regs[PCA9554_CONFIG];
+uint8_t output = s->regs[PCA9554_OUTPUT];
+uint8_t internal_state = config | output;
+
+for (i = 0; i < PCA9554_PIN_COUNT; i++) {
+uint8_t bit_mask = 1 << i;
+uint8_t internal_pin_state = (internal_state >> i) & 0x1;
+uint8_t old_value = s->regs[PCA9554_INPUT] & bit_mask;
+uint8_t new_value;
+
+switch (internal_pin_state) {
+case PCA9554_PIN_LOW:
+s->regs[PCA9554_INPUT] &= ~bit_mask;
+break;
+case PCA9554_PIN_HIZ:
+/*
+ * pullup sets it to a logical 1 unless
+ * external device drives it low.
+ */
+if (s->ext_state[i] == PCA9554_PIN_LOW) {
+s->regs[PCA9554_INPUT] &= ~bit_mask;
+} else {
+s->regs[PCA9554_INPUT] |=  bit_mask;
+}
+break;
+default:
+break;
+}
+
+/* update irq state only if pin state changed */
+new_value = s->regs[PCA9554_INPUT] & bit_mask;
+if (new_value != old_value) {
+if (new_value) {
+/* changed from 0 to 1 */
+qemu_set_irq(s->gpio_out[i], 1);
+} else {
+/* changed from 1 to 0 */
+qemu_set_irq(s->gpio_out[i], 0);
+}
+}
+}
+}
+
+static uint8_t pca9554_read(PCA9554State *s, uint8_t reg)
+{
+switch (reg) {
+case PCA9554_INPUT:
+return s->regs[PCA9554_INPUT] ^ s->regs[PCA9554_POLARITY];
+case PCA9554_OUTPUT:
+case PCA9554_POLARITY:
+case PCA9554_CONFIG:
+return s->regs[reg];
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected read to register %d\n",
+  __func__, reg);
+return 0xFF;
+}
+}
+
+static void pca9554_write(PCA9554State *s, uint8_t reg, uint8_t data)
+{
+switch (reg) {
+case PCA9554_OUTPUT:
+case PCA9554_CONFIG:
+s->regs[reg] = data;
+pca9554_update_pin_input(s);
+break;
+case PCA9554_POLARITY:
+s->regs[reg] = data;
+break;
+case PCA9554_INPUT:
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "%s: unexpected write to register %d\n",
+  __func__, reg);
+}
+}
+
+static uint8_t pca9554_recv(I2CSlave *i2c)
+{
+PCA9554State *s = PCA9554(i2c);
+uint8_t ret;
+
+ret = pca9554_read(s, s->pointer & 0x3);
+
+return ret;
+}
+
+static int pca9554_send(I2CSlave *i2c, uint8_t 

[PATCH v5 6/9] ppc/pnv: Use resettable interface to reset child I2C buses

[Glenn Miles]

The QEMU I2C buses and devices use the resettable
interface for resetting while the PNV I2C controller
and parent buses and devices have not yet transitioned
to this new interface and use the old reset strategy.
This was preventing the I2C buses and devices wired
to the PNV I2C controller from being reset.

The short term fix for this is to have the PNV I2C
Controller's reset function explicitly call the resettable
interface function, bus_cold_reset(), on all child
I2C buses.

The long term fix should be to transition all PNV parent
devices and buses to use the resettable interface so that
all child buses and devices are automatically reset.

Reviewed-by: Cédric Le Goater 
Signed-off-by: Glenn Miles 
---

No change from previous version

 hw/ppc/pnv_i2c.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
index 656a48eebe..774946d6b2 100644
--- a/hw/ppc/pnv_i2c.c
+++ b/hw/ppc/pnv_i2c.c
@@ -629,6 +629,19 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void 
*fdt,
 return 0;
 }
 
+static void pnv_i2c_sys_reset(void *dev)
+{
+int port;
+PnvI2C *i2c = PNV_I2C(dev);
+
+pnv_i2c_reset(dev);
+
+/* reset all buses connected to this i2c controller */
+for (port = 0; port < i2c->num_busses; port++) {
+bus_cold_reset(BUS(i2c->busses[port]));
+}
+}
+
 static void pnv_i2c_realize(DeviceState *dev, Error **errp)
 {
 PnvI2C *i2c = PNV_I2C(dev);
@@ -654,7 +667,7 @@ static void pnv_i2c_realize(DeviceState *dev, Error **errp)
 
 fifo8_create(>fifo, PNV_I2C_FIFO_SIZE);
 
-qemu_register_reset(pnv_i2c_reset, dev);
+qemu_register_reset(pnv_i2c_sys_reset, dev);
 
 qdev_init_gpio_out(DEVICE(dev), >psi_irq, 1);
 }
-- 
2.31.1

[PATCH v5 4/9] ppc/pnv: Add pca9552 to powernv10-rainier for PCIe hotplug power control

[Glenn Miles]

The Power Hypervisor code expects to see a pca9552 device connected
to the 3rd PNV I2C engine on port 1 at I2C address 0x63 (or left-
justified address of 0xC6).  This is used by hypervisor code to
control PCIe slot power during hotplug events.

Reviewed-by: Cédric Le Goater 
Signed-off-by: Glenn Miles 
---

Changes from previous version:
  - Formatting change

 hw/ppc/Kconfig   |  1 +
 hw/ppc/pnv.c | 25 +
 include/hw/ppc/pnv.h |  1 +
 3 files changed, 27 insertions(+)

diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index 56f0475a8e..f77ca773cf 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -32,6 +32,7 @@ config POWERNV
 select XIVE
 select FDT_PPC
 select PCI_POWERNV
+select PCA9552
 
 config PPC405
 bool
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 08704ce695..d8d19fb065 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -790,6 +790,7 @@ static void pnv_init(MachineState *machine)
 const char *bios_name = machine->firmware ?: FW_FILE_NAME;
 PnvMachineState *pnv = PNV_MACHINE(machine);
 MachineClass *mc = MACHINE_GET_CLASS(machine);
+PnvMachineClass *pmc = PNV_MACHINE_GET_CLASS(machine);
 char *fw_filename;
 long fw_size;
 uint64_t chip_ram_start = 0;
@@ -979,6 +980,13 @@ static void pnv_init(MachineState *machine)
  */
 pnv->powerdown_notifier.notify = pnv_powerdown_notify;
 qemu_register_powerdown_notifier(>powerdown_notifier);
+
+/*
+ * Create/Connect any machine-specific I2C devices
+ */
+if (pmc->i2c_init) {
+pmc->i2c_init(pnv);
+}
 }
 
 /*
@@ -1879,6 +1887,21 @@ static void pnv_chip_power10_realize(DeviceState *dev, 
Error **errp)
   qdev_get_gpio_in(DEVICE(>psi),
PSIHB9_IRQ_SBE_I2C));
 }
+
+}
+
+static void pnv_rainier_i2c_init(PnvMachineState *pnv)
+{
+int i;
+for (i = 0; i < pnv->num_chips; i++) {
+Pnv10Chip *chip10 = PNV10_CHIP(pnv->chips[i]);
+
+/*
+ * Add a PCA9552 I2C device for PCIe hotplug control
+ * to engine 2, bus 1, address 0x63
+ */
+i2c_slave_create_simple(chip10->i2c[2].busses[1], "pca9552", 0x63);
+}
 }
 
 static uint32_t pnv_chip_power10_xscom_pcba(PnvChip *chip, uint64_t addr)
@@ -2286,9 +2309,11 @@ static void pnv_machine_power10_class_init(ObjectClass 
*oc, void *data)
 static void pnv_machine_p10_rainier_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
+PnvMachineClass *pmc = PNV_MACHINE_CLASS(oc);
 
 pnv_machine_p10_common_class_init(oc, data);
 mc->desc = "IBM PowerNV (Non-Virtualized) POWER10 Rainier";
+pmc->i2c_init = pnv_rainier_i2c_init;
 }
 
 static bool pnv_machine_get_hb(Object *obj, Error **errp)
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 7e5fef7c43..110ac9aace 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -76,6 +76,7 @@ struct PnvMachineClass {
 int compat_size;
 
 void (*dt_power_mgt)(PnvMachineState *pnv, void *fdt);
+void (*i2c_init)(PnvMachineState *pnv);
 };
 
 struct PnvMachineState {
-- 
2.31.1

[PATCH v5 2/9] misc/pca9552: Let external devices set pca9552 inputs

[Glenn Miles]

Allow external devices to drive pca9552 input pins by adding
input GPIO's to the model.  This allows a device to connect
its output GPIO's to the pca9552 input GPIO's.

In order for an external device to set the state of a pca9552
pin, the pin must first be configured for high impedance (LED
is off).  If the pca9552 pin is configured to drive the pin low
(LED is on), then external input will be ignored.

Here is a table describing the logical state of a pca9552 pin
given the state being driven by the pca9552 and an external device:

   PCA9552
   Configured
   State

  | Hi-Z | Low |
--+--+-+
  External   Hi-Z |  Hi  | Low |
  Device--+--+-+
  State  Low  |  Low | Low |
--+--+-+

Reviewed-by: Andrew Jeffery 
Signed-off-by: Glenn Miles 
---

No change from previous version

 hw/misc/pca9552.c | 50 +--
 include/hw/misc/pca9552.h |  3 ++-
 2 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index 445f56a9e8..fe876471c8 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -44,6 +44,8 @@ DECLARE_CLASS_CHECKERS(PCA955xClass, PCA955X,
 #define PCA9552_LED_OFF  0x1
 #define PCA9552_LED_PWM0 0x2
 #define PCA9552_LED_PWM1 0x3
+#define PCA9552_PIN_LOW  0x0
+#define PCA9552_PIN_HIZ  0x1
 
 static const char *led_state[] = {"on", "off", "pwm0", "pwm1"};
 
@@ -110,22 +112,27 @@ static void pca955x_update_pin_input(PCA955xState *s)
 
 for (i = 0; i < k->pin_count; i++) {
 uint8_t input_reg = PCA9552_INPUT0 + (i / 8);
-uint8_t input_shift = (i % 8);
+uint8_t bit_mask = 1 << (i % 8);
 uint8_t config = pca955x_pin_get_config(s, i);
+uint8_t old_value = s->regs[input_reg] & bit_mask;
+uint8_t new_value;
 
 switch (config) {
 case PCA9552_LED_ON:
 /* Pin is set to 0V to turn on LED */
-qemu_set_irq(s->gpio[i], 0);
-s->regs[input_reg] &= ~(1 << input_shift);
+s->regs[input_reg] &= ~bit_mask;
 break;
 case PCA9552_LED_OFF:
 /*
  * Pin is set to Hi-Z to turn off LED and
- * pullup sets it to a logical 1.
+ * pullup sets it to a logical 1 unless
+ * external device drives it low.
  */
-qemu_set_irq(s->gpio[i], 1);
-s->regs[input_reg] |= 1 << input_shift;
+if (s->ext_state[i] == PCA9552_PIN_LOW) {
+s->regs[input_reg] &= ~bit_mask;
+} else {
+s->regs[input_reg] |=  bit_mask;
+}
 break;
 case PCA9552_LED_PWM0:
 case PCA9552_LED_PWM1:
@@ -133,6 +140,12 @@ static void pca955x_update_pin_input(PCA955xState *s)
 default:
 break;
 }
+
+/* update irq state only if pin state changed */
+new_value = s->regs[input_reg] & bit_mask;
+if (new_value != old_value) {
+qemu_set_irq(s->gpio_out[i], !!new_value);
+}
 }
 }
 
@@ -340,6 +353,7 @@ static const VMStateDescription pca9552_vmstate = {
 VMSTATE_UINT8(len, PCA955xState),
 VMSTATE_UINT8(pointer, PCA955xState),
 VMSTATE_UINT8_ARRAY(regs, PCA955xState, PCA955X_NR_REGS),
+VMSTATE_UINT8_ARRAY(ext_state, PCA955xState, PCA955X_PIN_COUNT_MAX),
 VMSTATE_I2C_SLAVE(i2c, PCA955xState),
 VMSTATE_END_OF_LIST()
 }
@@ -358,6 +372,7 @@ static void pca9552_reset(DeviceState *dev)
 s->regs[PCA9552_LS2] = 0x55;
 s->regs[PCA9552_LS3] = 0x55;
 
+memset(s->ext_state, PCA9552_PIN_HIZ, PCA955X_PIN_COUNT_MAX);
 pca955x_update_pin_input(s);
 
 s->pointer = 0xFF;
@@ -380,6 +395,26 @@ static void pca955x_initfn(Object *obj)
 }
 }
 
+static void pca955x_set_ext_state(PCA955xState *s, int pin, int level)
+{
+if (s->ext_state[pin] != level) {
+uint16_t pins_status = pca955x_pins_get_status(s);
+s->ext_state[pin] = level;
+pca955x_update_pin_input(s);
+pca955x_display_pins_status(s, pins_status);
+}
+}
+
+static void pca955x_gpio_in_handler(void *opaque, int pin, int level)
+{
+
+PCA955xState *s = PCA955X(opaque);
+PCA955xClass *k = PCA955X_GET_CLASS(s);
+
+assert((pin >= 0) && (pin < k->pin_count));
+pca955x_set_ext_state(s, pin, level);
+}
+
 static void pca955x_realize(DeviceState *dev, Error **errp)
 {
 PCA955xClass *k = PCA955X_GET_CLASS(dev);
@@ -389,7 +424,8 @@ static void pca955x_realize(DeviceState *dev, Error **errp)
 s->description = g_strdup("pca-unspecified");
 }
 
-qdev_init_gpio_out(dev, s->gpio, k->pin_count);
+qdev_init_gpio_out(dev, s->gpio_out, k->pin_count);
+qdev_init_gpio_in(dev, pca955x_gpio_in_handler, k->pin_count);
 }
 
 static Property pca955x_properties[] = {
diff --git a/include/hw/misc/pca9552.h 

[PATCH v5 1/9] misc/pca9552: Fix inverted input status

[Glenn Miles]

The pca9552 INPUT0 and INPUT1 registers are supposed to
hold the logical values of the LED pins.  A logical 0
should be seen in the INPUT0/1 registers for a pin when
its corresponding LSn bits are set to 0, which is also
the state needed for turning on an LED in a typical
usage scenario.  Existing code was doing the opposite
and setting INPUT0/1 bit to a 1 when the LSn bit was
set to 0, so this commit fixes that.

Reviewed-by: Andrew Jeffery 
Signed-off-by: Glenn Miles 
---

No change from previous version

 hw/misc/pca9552.c  | 18 +-
 tests/qtest/pca9552-test.c |  6 +++---
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
index fff19e369a..445f56a9e8 100644
--- a/hw/misc/pca9552.c
+++ b/hw/misc/pca9552.c
@@ -36,7 +36,10 @@ typedef struct PCA955xClass PCA955xClass;
 
 DECLARE_CLASS_CHECKERS(PCA955xClass, PCA955X,
TYPE_PCA955X)
-
+/*
+ * Note:  The LED_ON and LED_OFF configuration values for the PCA955X
+ *chips are the reverse of the PCA953X family of chips.
+ */
 #define PCA9552_LED_ON   0x0
 #define PCA9552_LED_OFF  0x1
 #define PCA9552_LED_PWM0 0x2
@@ -112,13 +115,18 @@ static void pca955x_update_pin_input(PCA955xState *s)
 
 switch (config) {
 case PCA9552_LED_ON:
-qemu_set_irq(s->gpio[i], 1);
-s->regs[input_reg] |= 1 << input_shift;
-break;
-case PCA9552_LED_OFF:
+/* Pin is set to 0V to turn on LED */
 qemu_set_irq(s->gpio[i], 0);
 s->regs[input_reg] &= ~(1 << input_shift);
 break;
+case PCA9552_LED_OFF:
+/*
+ * Pin is set to Hi-Z to turn off LED and
+ * pullup sets it to a logical 1.
+ */
+qemu_set_irq(s->gpio[i], 1);
+s->regs[input_reg] |= 1 << input_shift;
+break;
 case PCA9552_LED_PWM0:
 case PCA9552_LED_PWM1:
 /* TODO */
diff --git a/tests/qtest/pca9552-test.c b/tests/qtest/pca9552-test.c
index d80ed93cd3..ccca2b3d91 100644
--- a/tests/qtest/pca9552-test.c
+++ b/tests/qtest/pca9552-test.c
@@ -60,7 +60,7 @@ static void send_and_receive(void *obj, void *data, 
QGuestAllocator *alloc)
 g_assert_cmphex(value, ==, 0x55);
 
 value = i2c_get8(i2cdev, PCA9552_INPUT0);
-g_assert_cmphex(value, ==, 0x0);
+g_assert_cmphex(value, ==, 0xFF);
 
 pca9552_init(i2cdev);
 
@@ -68,13 +68,13 @@ static void send_and_receive(void *obj, void *data, 
QGuestAllocator *alloc)
 g_assert_cmphex(value, ==, 0x54);
 
 value = i2c_get8(i2cdev, PCA9552_INPUT0);
-g_assert_cmphex(value, ==, 0x01);
+g_assert_cmphex(value, ==, 0xFE);
 
 value = i2c_get8(i2cdev, PCA9552_LS3);
 g_assert_cmphex(value, ==, 0x54);
 
 value = i2c_get8(i2cdev, PCA9552_INPUT1);
-g_assert_cmphex(value, ==, 0x10);
+g_assert_cmphex(value, ==, 0xEF);
 }
 
 static void pca9552_register_nodes(void)
-- 
2.31.1

[PATCH v5 8/9] ppc/pnv: Add a pca9554 I2C device to powernv10-rainier

[Glenn Miles]

For powernv10-rainier, the Power Hypervisor code expects to see a
pca9554 device connected to the 3rd PNV I2C engine on port 1 at I2C
address 0x25 (or left-justified address of 0x4A).  This is used by
the hypervisor code to detect if a "Cable Card" is present.

Signed-off-by: Glenn Miles 
---

No change from previous version

 hw/misc/Kconfig | 4 
 hw/misc/meson.build | 1 +
 hw/ppc/Kconfig  | 1 +
 hw/ppc/pnv.c| 6 ++
 4 files changed, 12 insertions(+)

diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index cc8a8c1418..c347a132c2 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -34,6 +34,10 @@ config PCA9552
 bool
 depends on I2C
 
+config PCA9554
+bool
+depends on I2C
+
 config I2C_ECHO
 bool
 default y if TEST_DEVICES
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 36c20d5637..c39410e4a7 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_FW_CFG_DMA', if_true: 
files('vmcoreinfo.c'))
 system_ss.add(when: 'CONFIG_ISA_DEBUG', if_true: files('debugexit.c'))
 system_ss.add(when: 'CONFIG_ISA_TESTDEV', if_true: files('pc-testdev.c'))
 system_ss.add(when: 'CONFIG_PCA9552', if_true: files('pca9552.c'))
+system_ss.add(when: 'CONFIG_PCA9554', if_true: files('pca9554.c'))
 system_ss.add(when: 'CONFIG_PCI_TESTDEV', if_true: files('pci-testdev.c'))
 system_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c'))
 system_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c'))
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index f77ca773cf..2302778265 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -33,6 +33,7 @@ config POWERNV
 select FDT_PPC
 select PCI_POWERNV
 select PCA9552
+select PCA9554
 
 config PPC405
 bool
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 088824fd9f..1dab7c57e8 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1918,6 +1918,12 @@ static void pnv_rainier_i2c_init(PnvMachineState *pnv)
   qdev_get_gpio_in(DEVICE(hotplug), 8));
 qdev_connect_gpio_out(DEVICE(hotplug), 4,
   qdev_get_gpio_in(DEVICE(hotplug), 9));
+
+/*
+ * Add a PCA9554 I2C device for cable card presence detection
+ * to engine 2, bus 1, address 0x25
+ */
+i2c_slave_create_simple(chip10->i2c[2].busses[1], "pca9554", 0x25);
 }
 }
 
-- 
2.31.1

[PATCH v5 0/9] Add powernv10 I2C devices and tests

[Glenn Miles]

This series of patches includes support, tests and fixes for
adding PCA9552 and PCA9554 I2C devices to the powernv10 chip.

The PCA9552 device is used for PCIe slot hotplug power control
and monitoring, while the PCA9554 device is used for presence
detection of IBM CableCard devices.  Both devices are required
by the Power Hypervisor Firmware on the Power10 Ranier platform.

Changes from previous version:
  - Removed two already merged patches
  - Various formatting changes
  - Capitalized "Rainier" in machine description string
  - Changed powernv10-rainier parent to MACHINE_TYPE_NAME("powernv10")


Glenn Miles (9):
  misc/pca9552: Fix inverted input status
  misc/pca9552: Let external devices set pca9552 inputs
  ppc/pnv: New powernv10-rainier machine type
  ppc/pnv: Add pca9552 to powernv10-rainier for PCIe hotplug power
control
  ppc/pnv: Wire up pca9552 GPIO pins for PCIe hotplug power control
  ppc/pnv: Use resettable interface to reset child I2C buses
  misc: Add a pca9554 GPIO device model
  ppc/pnv: Add a pca9554 I2C device to powernv10-rainier
  ppc/pnv: Test pnv i2c master and connected devices

 MAINTAINERS |  10 +-
 hw/misc/Kconfig |   4 +
 hw/misc/meson.build |   1 +
 hw/misc/pca9552.c   |  58 ++-
 hw/misc/pca9554.c   | 328 
 hw/ppc/Kconfig  |   2 +
 hw/ppc/pnv.c|  72 +++-
 hw/ppc/pnv_i2c.c|  15 +-
 include/hw/misc/pca9552.h   |   3 +-
 include/hw/misc/pca9554.h   |  36 ++
 include/hw/misc/pca9554_regs.h  |  19 +
 include/hw/ppc/pnv.h|   1 +
 tests/qtest/meson.build |   1 +
 tests/qtest/pca9552-test.c  |   6 +-
 tests/qtest/pnv-host-i2c-test.c | 650 
 15 files changed, 1190 insertions(+), 16 deletions(-)
 create mode 100644 hw/misc/pca9554.c
 create mode 100644 include/hw/misc/pca9554.h
 create mode 100644 include/hw/misc/pca9554_regs.h
 create mode 100644 tests/qtest/pnv-host-i2c-test.c

-- 
2.31.1

Re: [PATCH-for-9.0 04/25] memory: Simplify memory_region_init_rom_nomigrate() calls

[Richard Henderson]

On 11/21/23 12:50, Richard Henderson wrote:

On 11/20/23 15:32, Philippe Mathieu-Daudé wrote:

Mechanical change using the following coccinelle script:

@@
expression mr, owner, arg3, arg4, errp;
@@
-   memory_region_init_rom_nomigrate(mr, owner, arg3, arg4, );
 if (
-   errp
+   !memory_region_init_rom_nomigrate(mr, owner, arg3, arg4, )


This coccinelle script doesn't quite match...


@@ -3628,11 +3628,8 @@ void memory_region_init_rom(MemoryRegion *mr,
  Error **errp)
  {
  DeviceState *owner_dev;
-    Error *err = NULL;
-    memory_region_init_rom_nomigrate(mr, owner, name, size, );


... this?


I'm sorry, it does.  "expression errp" can match "err".
It's the manual local variable removal that threw me off.


r~

Re: [PATCH-for-9.0 04/25] memory: Simplify memory_region_init_rom_nomigrate() calls

[Richard Henderson]

On 11/20/23 15:32, Philippe Mathieu-Daudé wrote:

Mechanical change using the following coccinelle script:

@@
expression mr, owner, arg3, arg4, errp;
@@
-   memory_region_init_rom_nomigrate(mr, owner, arg3, arg4, );
 if (
-   errp
+   !memory_region_init_rom_nomigrate(mr, owner, arg3, arg4, )


This coccinelle script doesn't quite match...


@@ -3628,11 +3628,8 @@ void memory_region_init_rom(MemoryRegion *mr,
  Error **errp)
  {
  DeviceState *owner_dev;
-Error *err = NULL;
  
-memory_region_init_rom_nomigrate(mr, owner, name, size, );


... this?

That said, the actual code change is good.

Reviewed-by: Richard Henderson 


r~

Re: [PATCH for-8.2 0/2] qdev array property fixes

[Thomas Huth]

On 21/11/2023 18.34, Kevin Wolf wrote:

Kevin Wolf (2):
   qdev: Fix crash in array property getter
   string-output-visitor: Support lists for non-integer types

  hw/core/qdev-properties.c| 33 ++---
  qapi/string-output-visitor.c | 24 
  2 files changed, 46 insertions(+), 11 deletions(-)


Thanks, I can confirm that this series fixes the problem for me - both 
locally when compiling with CFI, and on Travis (where I observed the problem 
first):


 https://app.travis-ci.com/github/huth/qemu/jobs/613659935#L6580

Tested-by: Thomas Huth 

Re: [PATCH] target/arm: Don't implement *32_EL2 registers when EL1 is AArch64 only

[Richard Henderson]

On 11/21/23 08:46, Peter Maydell wrote:

The system registers DBGVCR32_EL2, FPEXC32_EL2, DACR32_EL2 and
IFSR32_EL2 are present only to allow an AArch64 EL2 or EL3 to read
and write the contents of an AArch32-only system register.  The
architecture requires that they are present only when EL1 can be
AArch32, but we implement them unconditionally.  This was OK when all
our CPUs supported AArch32 EL1, but we have quite a lot of CPU models
now which only support AArch64 at EL1:
  a64fx
  cortex-a76
  cortex-a710
  neoverse-n1
  neoverse-n2
  neoverse-v1

Only define these registers for CPUs which allow AArch32 EL1.

Signed-off-by: Peter Maydell 
---
I happened to notice this reading through the Arm ARM recently.  This
is technically a bug, but you'll only notice it if you deliberately
look at what should be an unimplemented register to see if it UNDEFs,
so I don't think it's worth either putting in 8.2 or backporting to
stable.
---
  target/arm/debug_helper.c | 23 +++
  target/arm/helper.c   | 35 +--
  2 files changed, 36 insertions(+), 22 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [PATCH v7 08/27] vfio/pci: Introduce a vfio pci hot reset interface

[Philippe Mathieu-Daudé]

Hi Zhenzhong,

On 21/11/23 09:44, Zhenzhong Duan wrote:

Legacy vfio pci and iommufd cdev have different process to hot reset
vfio device, expand current code to abstract out pci_hot_reset callback
for legacy vfio, this same interface will also be used by iommufd
cdev vfio device.

Rename vfio_pci_hot_reset to vfio_legacy_pci_hot_reset and move it
into container.c.

vfio_pci_[pre/post]_reset and vfio_pci_host_match are exported so
they could be called in legacy and iommufd pci_hot_reset callback.

Suggested-by: Cédric Le Goater 
Signed-off-by: Zhenzhong Duan 
Reviewed-by: Eric Auger 
Tested-by: Eric Auger 
---
  hw/vfio/pci.h |   3 +
  include/hw/vfio/vfio-container-base.h |   3 +
  hw/vfio/container.c   | 170 ++
  hw/vfio/pci.c | 168 +
  4 files changed, 182 insertions(+), 162 deletions(-)




@@ -2485,166 +2485,10 @@ int vfio_pci_get_pci_hot_reset_info(VFIOPCIDevice 
*vdev,
  
  static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)

  {



+return ops->pci_hot_reset(vbasedev, single);


At this point vfio_iommufd_ops.pci_hot_reset is NULL.
Worth checking for non-NULL before calling.


  }
  
  /*

Re: [PATCH v4 05/11] ppc/pnv: Wire up pca9552 GPIO pins for PCIe hotplug power control

[Cédric Le Goater]

On 11/21/23 00:51, Glenn Miles wrote:

For power10-rainier, a pca9552 device is used for PCIe slot hotplug
power control by the Power Hypervisor code.  The code expects that
some time after it enables power to a PCIe slot by asserting one of
the pca9552 GPIO pins 0-4, it should see a "power good" signal asserted
on one of pca9552 GPIO pins 5-9.


And this is what OPAL is not doing :

  
https://github.com/open-power/skiboot/blob/master/platforms/astbmc/rainier.c#L65

Correct ?


To simulate this behavior, we simply connect the GPIO outputs for
pins 0-4 to the GPIO inputs for pins 5-9.

Each PCIe slot is assigned 3 GPIO pins on the pca9552 device, for
control of up to 5 PCIe slots.  The per-slot signal names are:

SLOTx_EN...PHYP uses this as an output to enable
   slot power.  We connect this to the
   SLOTx_PG pin to simulate a PGOOD signal.
SLOTx_PG...PHYP uses this as in input to detect
   PGOOD for the slot.  For our purposes
   we just connect this to the SLOTx_EN
   output.
SLOTx_Control..PHYP uses this as an output to prevent
   a race condition in the real hotplug
   circuitry, but we can ignore this output
   for simulation.

Signed-off-by: Glenn Miles 
---

Changes from previous version:
   - Code moved from pnv_chip_power10_realize to pnv_rainier_i2c_init

  hw/ppc/pnv.c | 20 ++--
  1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 9cefcd0fd6..80d25fc1bd 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1898,8 +1898,24 @@ static void pnv_rainier_i2c_init(PnvMachineState *pnv)
   * Add a PCA9552 I2C device for PCIe hotplug control
   * to engine 2, bus 1, address 0x63
   */
-i2c_slave_create_simple(chip10->i2c[2].busses[1],
-"pca9552", 0x63);
+I2CSlave *hotplug = i2c_slave_create_simple(chip10->i2c[2].busses[1],
+"pca9552", 0x63);


hotplug ? why not dev simply ?


Thanks,

C.




+
+/*
+ * Connect PCA9552 GPIO pins 0-4 (SLOTx_EN) outputs to GPIO pins 5-9
+ * (SLOTx_PG) inputs in order to fake the pgood state of PCIe slots
+ * after hypervisor code sets a SLOTx_EN pin high.
+ */
+qdev_connect_gpio_out(DEVICE(hotplug), 0,
+  qdev_get_gpio_in(DEVICE(hotplug), 5));
+qdev_connect_gpio_out(DEVICE(hotplug), 1,
+  qdev_get_gpio_in(DEVICE(hotplug), 6));
+qdev_connect_gpio_out(DEVICE(hotplug), 2,
+  qdev_get_gpio_in(DEVICE(hotplug), 7));
+qdev_connect_gpio_out(DEVICE(hotplug), 3,
+  qdev_get_gpio_in(DEVICE(hotplug), 8));
+qdev_connect_gpio_out(DEVICE(hotplug), 4,
+  qdev_get_gpio_in(DEVICE(hotplug), 9));
  }
  }
  

Re: [PATCH v4 03/11] ppc/pnv: New powernv10-rainier machine type

[Miles Glenn]

On Tue, 2023-11-21 at 19:26 +0100, Cédric Le Goater wrote:
> On 11/21/23 17:36, Miles Glenn wrote:
> > On Tue, 2023-11-21 at 08:29 +0100, Cédric Le Goater wrote:
> > > On 11/21/23 02:33, Nicholas Piggin wrote:
> > > > On Tue Nov 21, 2023 at 9:51 AM AEST, Glenn Miles wrote:
> > > > > Create a new powernv machine type, powernv10-rainier, that
> > > > > will contain rainier-specific devices.
> > > > 
> > > > Is the plan to have a base powernv10 common to all and then
> > > > powernv10-rainier looks like a Rainier? Or would powernv10
> > > > just be a rainier?
> > > > 
> > > > It's fine to structure code this way, I'm just wondering about
> > > > the machine types available to user. Is a base powernv10
> > > > machine
> > > > useful to run?
> > > 
> > > There are multiple P10 boards defined in Linux :
> > > 
> > > aspeed-bmc-ibm-bonnell.dts
> > > aspeed-bmc-ibm-everest.dts
> > > aspeed-bmc-ibm-rainier-1s4u.dts
> > > aspeed-bmc-ibm-rainier-4u.dts
> > > aspeed-bmc-ibm-rainier.dts
> > > 
> > > and we could model the machines above with a fixed number of
> > > sockets.
> > > The "powernv10" would be the generic system that can be
> > > customized
> > > at will on the command line, even I2C devices. There is also the
> > > P10 Denali which is FSP based. This QEMU machine would certainly
> > > be
> > > very different. I thought of doing the same for P9 with a -zaius
> > > and include NPU2 models for it. I lacked time and the interest
> > > was
> > > small at the time of OpenPOWER.
> > > 
> > > Anyhow, adding a new machine makes sense and it prepares ground
> > > for
> > > possible new ones. I am OK with or without. As primary users, you
> > > are
> > > the ones that can tell if there will be a second machine.
> > >
> > > Thanks,
> > > 
> > > C.
> > > 
> > 
> > I am not sure what the powernv10 machine would be used for.  The
> > only reason I kept it was because I didn't want to break anyone out
> > there that might be using it.
> (previous email sent to fast)
> 
> You would need to go through the deprecation process [1] if you want
> to remove the machine. I suggest keeping it for now since it is two
> lines of type definition.
> 

Yes, I agree.

> > My preference would have been to just make powernv10-rainier an
> > alias of the powernv10 machine, but only one alias name per machine
> > is supported and there is already a plan to make "powernv" an
> > alias for the powernv10 machine.
> 
> yes. It might be time now for PowerNV and pSeries to update the
> default processor. Let's address that in the QEMU 9.0 cycle.
> 
> Thanks,
> 
> C.
> 
> [1] https://qemu.readthedocs.io/en/v8.1.0/about/deprecated.html
> 

Sounds good, thanks!

Glenn

Re: [PATCH-for-8.2? 2/6] hw/virtio: Free VirtIOIOMMUPCI::vdev.reserved_regions[] on finalize()

[Eric Auger]

Hi Phil,

On 11/21/23 18:40, Philippe Mathieu-Daudé wrote:
> Commit 0be6bfac62 ("qdev: Implement variable length array properties")
> added the DEFINE_PROP_ARRAY() macro with the following comment:
>
>   * It is the responsibility of the device deinit code to free the
>   * @_arrayfield memory.
>
> Commit 8077b8e549 added:
>
>   DEFINE_PROP_ARRAY("reserved-regions", VirtIOIOMMUPCI,
> vdev.nb_reserved_regions, vdev.reserved_regions,
> qdev_prop_reserved_region, ReservedRegion),
>
> but forgot to free the 'vdev.reserved_regions' array. Do it in the
> instance_finalize() handler.
>
> Cc: qemu-sta...@nongnu.org
> Fixes: 8077b8e549 ("virtio-iommu-pci: Add array of Interval properties") # 
> v5.1.0+
> Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Auger 

Eric
> ---
>  hw/virtio/virtio-iommu-pci.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
> index 9459fbf6ed..cbdfe4c591 100644
> --- a/hw/virtio/virtio-iommu-pci.c
> +++ b/hw/virtio/virtio-iommu-pci.c
> @@ -95,10 +95,18 @@ static void virtio_iommu_pci_instance_init(Object *obj)
>  TYPE_VIRTIO_IOMMU);
>  }
>  
> +static void virtio_iommu_pci_instance_finalize(Object *obj)
> +{
> +VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
> +
> +g_free(dev->vdev.prop_resv_regions);
> +}
> +
>  static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
>  .generic_name  = TYPE_VIRTIO_IOMMU_PCI,
>  .instance_size = sizeof(VirtIOIOMMUPCI),
>  .instance_init = virtio_iommu_pci_instance_init,
> +.instance_finalize = virtio_iommu_pci_instance_finalize,
>  .class_init= virtio_iommu_pci_class_init,
>  };
>  

Re: [PATCH v3] hw/i386: fix short-circuit logic with non-optimizing builds

[Dan Hoffman]

I'm writing a patch to clang's constant folding to address this case
(doesn't seem too difficult). I'll either follow up with a link to
some submissions I've made or a bug report on the project describing
the issue.



On Tue, Nov 21, 2023 at 10:15 AM Eric Blake  wrote:
>
> On Mon, Nov 20, 2023 at 11:20:52AM +0100, Philippe Mathieu-Daudé wrote:
> > (Cc'ing Eric)
> >
> > On 20/11/23 10:28, Michael S. Tsirkin wrote:
> > > On Sun, Nov 19, 2023 at 07:34:58PM -0600, Dan Hoffman wrote:
> > > > As far as I can tell, yes. Any optimization level above O0 does not 
> > > > have this
> > > > issue (on this version of Clang, at least)
> > >
> > > Aha, this is with -O0. That makes sense.
> >
> > But then, why the other cases aren't problematic?
> >
> > $ git grep -E ' (&&|\|\|) !?kvm_enabled'
> > hw/arm/boot.c:1228:assert(!(info->secure_board_setup && kvm_enabled()));
>
> This one's obvious; no kvm_*() calls in the assert.
>
> > hw/i386/microvm.c:270:(mms->rtc == ON_OFF_AUTO_AUTO &&
> > !kvm_enabled())) {
>
> Ones like this require reading context to see whether the if() block
> guarded any kvm_*() calls for the linker to elide - but still a fairly
> easy audit.
>
> > > >
> > > >  I'm surprised the order of conditions matters for code elision...
> > > >
> > > >  > Signed-off-by: Daniel Hoffman 
> > > >  > ---
> > > >  >   hw/i386/x86.c | 15 ---
> > > >  >   1 file changed, 12 insertions(+), 3 deletions(-)
> > > >  >
> > > >  > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > > >  > index b3d054889bb..2b6291ad8d5 100644
> > > >  > --- a/hw/i386/x86.c
> > > >  > +++ b/hw/i386/x86.c
> > > >  > @@ -131,8 +131,12 @@ void x86_cpus_init(X86MachineState *x86ms, 
> > > > int
> > > >  default_cpu_version)
> > > >  >   /*
> > > >  >* Can we support APIC ID 255 or higher?  With KVM, that 
> > > > requires
> > > >  >* both in-kernel lapic and X2APIC userspace API.
> > > >  > + *
> > > >  > + * kvm_enabled() must go first to ensure that kvm_* 
> > > > references are
> > > >  > + * not emitted for the linker to consume (kvm_enabled() is
> > > >  > + * a literal `0` in configurations where kvm_* aren't 
> > > > defined)
> > > >  >*/
> > > >  > -if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
> > > >  > +if (kvm_enabled() && x86ms->apic_id_limit > 255 &&
> > > >  >   (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
>
> Indeed, if clang -O0 treats 'if (cond1 && 0 && cond2)' differently
> than 'if (0 && cond1 && cond2)' for purposes of eliding the code for
> cond2, that seems like a blatant missed optimization that we should be
> reporting to the clang folks.
>
> While this patch may solve the immediate issue, it does not scale - if
> we ever have two separate guards that can both be independently
> hard-coded to 0 based on configure-time decisions, but which are both
> used as guards in the same expression, it will become impossible to
> avoid a '(cond1 && 0 && cond2)' scenario across all 4 possible
> configurations of those two guards.
>
> I have no objection to the patch, but it would be nice if the commit
> message could point to a clang bug report, if one has been filed.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.
> Virtualization:  qemu.org | libguestfs.org
>

Re: [PATCH v4 03/11] ppc/pnv: New powernv10-rainier machine type

[Cédric Le Goater]

On 11/21/23 17:36, Miles Glenn wrote:

On Tue, 2023-11-21 at 08:29 +0100, Cédric Le Goater wrote:

On 11/21/23 02:33, Nicholas Piggin wrote:

On Tue Nov 21, 2023 at 9:51 AM AEST, Glenn Miles wrote:

Create a new powernv machine type, powernv10-rainier, that
will contain rainier-specific devices.


Is the plan to have a base powernv10 common to all and then
powernv10-rainier looks like a Rainier? Or would powernv10
just be a rainier?

It's fine to structure code this way, I'm just wondering about
the machine types available to user. Is a base powernv10 machine
useful to run?


There are multiple P10 boards defined in Linux :

aspeed-bmc-ibm-bonnell.dts
aspeed-bmc-ibm-everest.dts
aspeed-bmc-ibm-rainier-1s4u.dts
aspeed-bmc-ibm-rainier-4u.dts
aspeed-bmc-ibm-rainier.dts

and we could model the machines above with a fixed number of sockets.
The "powernv10" would be the generic system that can be customized
at will on the command line, even I2C devices. There is also the
P10 Denali which is FSP based. This QEMU machine would certainly be
very different. I thought of doing the same for P9 with a -zaius
and include NPU2 models for it. I lacked time and the interest was
small at the time of OpenPOWER.

Anyhow, adding a new machine makes sense and it prepares ground for
possible new ones. I am OK with or without. As primary users, you are
the ones that can tell if there will be a second machine.
   
Thanks,


C.



I am not sure what the powernv10 machine would be used for.  The
only reason I kept it was because I didn't want to break anyone out
there that might be using it.

(previous email sent to fast)

You would need to go through the deprecation process [1] if you want
to remove the machine. I suggest keeping it for now since it is two
lines of type definition.


My preference would have been to just make powernv10-rainier an
alias of the powernv10 machine, but only one alias name per machine
is supported and there is already a plan to make "powernv" an
alias for the powernv10 machine.


yes. It might be time now for PowerNV and pSeries to update the
default processor. Let's address that in the QEMU 9.0 cycle.

Thanks,

C.

[1] https://qemu.readthedocs.io/en/v8.1.0/about/deprecated.html

Re: [PATCH v4 08/11] ppc/pnv: Use resettable interface to reset child I2C buses

[Cédric Le Goater]

On 11/21/23 00:51, Glenn Miles wrote:

The QEMU I2C buses and devices use the resettable
interface for resetting while the PNV I2C controller
and parent buses and devices have not yet transitioned
to this new interface and use the old reset strategy.
This was preventing the I2C buses and devices wired
to the PNV I2C controller from being reset.

The short term fix for this is to have the PNV I2C
Controller's reset function explicitly call the resettable
interface function, bus_cold_reset(), on all child
I2C buses.

The long term fix should be to transition all PNV parent
devices and buses to use the resettable interface so that
all child buses and devices are automatically reset.

Signed-off-by: Glenn Miles 
---



Reviewed-by: Cédric Le Goater 

Thanks,

C.




No changes from previous version

  hw/ppc/pnv_i2c.c | 15 ++-
  1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
index f80589157b..9ced596b98 100644
--- a/hw/ppc/pnv_i2c.c
+++ b/hw/ppc/pnv_i2c.c
@@ -628,6 +628,19 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void 
*fdt,
  return 0;
  }
  
+static void pnv_i2c_sys_reset(void *dev)

+{
+int port;
+PnvI2C *i2c = PNV_I2C(dev);
+
+pnv_i2c_reset(dev);
+
+/* reset all buses connected to this i2c controller */
+for (port = 0; port < i2c->num_busses; port++) {
+bus_cold_reset(BUS(i2c->busses[port]));
+}
+}
+
  static void pnv_i2c_realize(DeviceState *dev, Error **errp)
  {
  PnvI2C *i2c = PNV_I2C(dev);
@@ -648,7 +661,7 @@ static void pnv_i2c_realize(DeviceState *dev, Error **errp)
  
  fifo8_create(>fifo, PNV_I2C_FIFO_SIZE);
  
-qemu_register_reset(pnv_i2c_reset, dev);

+qemu_register_reset(pnv_i2c_sys_reset, dev);
  
  qdev_init_gpio_out(DEVICE(dev), >psi_irq, 1);

  }

Re: [PATCH v4 07/11] ppc/pnv: Fix PNV I2C invalid status after reset

[Cédric Le Goater]

On 11/21/23 00:51, Glenn Miles wrote:

The PNV I2C Controller was clearing the status register
after a reset without repopulating the "upper threshold
for I2C ports", "Command Complete" and the SCL/SDA input
level fields.

Fixed this for resets caused by a system reset as well
as from writing to the "Immediate Reset" register.

Reviewed-by: Cédric Le Goater 
Fixes: 263b81ee15af ("ppc/pnv: Add an I2C controller model")
Signed-off-by: Glenn Miles 
---

No changes from previous version


This patch was merged upstream now.

C.






  hw/ppc/pnv_i2c.c | 42 ++
  1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
index b2c738da50..f80589157b 100644
--- a/hw/ppc/pnv_i2c.c
+++ b/hw/ppc/pnv_i2c.c
@@ -462,6 +462,23 @@ static uint64_t pnv_i2c_xscom_read(void *opaque, hwaddr 
addr,
  return val;
  }
  
+static void pnv_i2c_reset(void *dev)

+{
+PnvI2C *i2c = PNV_I2C(dev);
+
+memset(i2c->regs, 0, sizeof(i2c->regs));
+
+i2c->regs[I2C_STAT_REG] =
+SETFIELD(I2C_STAT_UPPER_THRS, 0ull, i2c->num_busses - 1) |
+I2C_STAT_CMD_COMP | I2C_STAT_SCL_INPUT_LEVEL |
+I2C_STAT_SDA_INPUT_LEVEL;
+i2c->regs[I2C_EXTD_STAT_REG] =
+SETFIELD(I2C_EXTD_STAT_FIFO_SIZE, 0ull, PNV_I2C_FIFO_SIZE) |
+SETFIELD(I2C_EXTD_STAT_I2C_VERSION, 0ull, 23); /* last version */
+
+fifo8_reset(>fifo);
+}
+
  static void pnv_i2c_xscom_write(void *opaque, hwaddr addr,
  uint64_t val, unsigned size)
  {
@@ -499,16 +516,7 @@ static void pnv_i2c_xscom_write(void *opaque, hwaddr addr,
  break;
  
  case I2C_RESET_I2C_REG:

-i2c->regs[I2C_MODE_REG] = 0;
-i2c->regs[I2C_CMD_REG] = 0;
-i2c->regs[I2C_WATERMARK_REG] = 0;
-i2c->regs[I2C_INTR_MASK_REG] = 0;
-i2c->regs[I2C_INTR_COND_REG] = 0;
-i2c->regs[I2C_INTR_RAW_COND_REG] = 0;
-i2c->regs[I2C_STAT_REG] = 0;
-i2c->regs[I2C_RESIDUAL_LEN_REG] = 0;
-i2c->regs[I2C_EXTD_STAT_REG] &=
-(I2C_EXTD_STAT_FIFO_SIZE | I2C_EXTD_STAT_I2C_VERSION);
+pnv_i2c_reset(i2c);
  break;
  
  case I2C_RESET_ERRORS:

@@ -620,20 +628,6 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void 
*fdt,
  return 0;
  }
  
-static void pnv_i2c_reset(void *dev)

-{
-PnvI2C *i2c = PNV_I2C(dev);
-
-memset(i2c->regs, 0, sizeof(i2c->regs));
-
-i2c->regs[I2C_STAT_REG] = I2C_STAT_CMD_COMP;
-i2c->regs[I2C_EXTD_STAT_REG] =
-SETFIELD(I2C_EXTD_STAT_FIFO_SIZE, 0ull, PNV_I2C_FIFO_SIZE) |
-SETFIELD(I2C_EXTD_STAT_I2C_VERSION, 0ull, 23); /* last version */
-
-fifo8_reset(>fifo);
-}
-
  static void pnv_i2c_realize(DeviceState *dev, Error **errp)
  {
  PnvI2C *i2c = PNV_I2C(dev);

Re: [PATCH v4 06/11] ppc/pnv: PNV I2C engines assigned incorrect XSCOM addresses

[Cédric Le Goater]

On 11/21/23 00:51, Glenn Miles wrote:

The PNV I2C engines for power9 and power10 were being assigned a base
XSCOM address that was off by one I2C engine's address range such
that engine 0 had engine 1's address and so on.  The xscom address
assignment was being based on the device tree engine numbering, which
starts at 1.  Rather than changing the device tree numbering to start
with 0, the addressing was changed to be based on the existing device
tree numbers minus one.

Reviewed-by: Cédric Le Goater 
Fixes: 1ceda19c28a1 ("ppc/pnv: Connect PNV I2C controller to powernv10)
Signed-off-by: Glenn Miles 
---

No changes from previous version


This patch was merged upstream now.

C.




  hw/ppc/pnv.c | 6 --
  hw/ppc/pnv_i2c.c | 2 +-
  2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 80d25fc1bd..c29a136465 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1631,7 +1631,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, 
Error **errp)
  return;
  }
  pnv_xscom_add_subregion(chip, PNV9_XSCOM_I2CM_BASE +
-   chip9->i2c[i].engine * PNV9_XSCOM_I2CM_SIZE,
+(chip9->i2c[i].engine - 1) *
+PNV9_XSCOM_I2CM_SIZE,
  >i2c[i].xscom_regs);
  qdev_connect_gpio_out(DEVICE(>i2c[i]), 0,
qdev_get_gpio_in(DEVICE(>psi),
@@ -1879,7 +1880,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, 
Error **errp)
  return;
  }
  pnv_xscom_add_subregion(chip, PNV10_XSCOM_I2CM_BASE +
-chip10->i2c[i].engine * PNV10_XSCOM_I2CM_SIZE,
+(chip10->i2c[i].engine - 1) *
+PNV10_XSCOM_I2CM_SIZE,
  >i2c[i].xscom_regs);
  qdev_connect_gpio_out(DEVICE(>i2c[i]), 0,
qdev_get_gpio_in(DEVICE(>psi),
diff --git a/hw/ppc/pnv_i2c.c b/hw/ppc/pnv_i2c.c
index f75e59e709..b2c738da50 100644
--- a/hw/ppc/pnv_i2c.c
+++ b/hw/ppc/pnv_i2c.c
@@ -593,7 +593,7 @@ static int pnv_i2c_dt_xscom(PnvXScomInterface *dev, void 
*fdt,
  int i2c_offset;
  const char i2c_compat[] = "ibm,power8-i2cm\0ibm,power9-i2cm";
  uint32_t i2c_pcba = PNV9_XSCOM_I2CM_BASE +
-i2c->engine * PNV9_XSCOM_I2CM_SIZE;
+(i2c->engine - 1) * PNV9_XSCOM_I2CM_SIZE;
  uint32_t reg[2] = {
  cpu_to_be32(i2c_pcba),
  cpu_to_be32(PNV9_XSCOM_I2CM_SIZE)

Re: [PATCH v4 10/11] ppc/pnv: Add a pca9554 I2C device to powernv10-rainier

[Cédric Le Goater]

On 11/21/23 00:51, Glenn Miles wrote:

For powernv10-rainier, the Power Hypervisor code expects to see a
pca9554 device connected to the 3rd PNV I2C engine on port 1 at I2C
address 0x25 (or left-justified address of 0x4A).  This is used by
the hypervisor code to detect if a "Cable Card" is present.

Signed-off-by: Glenn Miles 



Reviewed-by: Cédric Le Goater 

Thanks,

C.



---

Changes from previous version:
   - Code moved from pnv_chip_power10_realize to pnv_rainier_i2c_init

  hw/misc/Kconfig | 4 
  hw/misc/meson.build | 1 +
  hw/ppc/Kconfig  | 1 +
  hw/ppc/pnv.c| 6 ++
  4 files changed, 12 insertions(+)

diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index cc8a8c1418..c347a132c2 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -34,6 +34,10 @@ config PCA9552
  bool
  depends on I2C
  
+config PCA9554

+bool
+depends on I2C
+
  config I2C_ECHO
  bool
  default y if TEST_DEVICES
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 36c20d5637..c39410e4a7 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -4,6 +4,7 @@ system_ss.add(when: 'CONFIG_FW_CFG_DMA', if_true: 
files('vmcoreinfo.c'))
  system_ss.add(when: 'CONFIG_ISA_DEBUG', if_true: files('debugexit.c'))
  system_ss.add(when: 'CONFIG_ISA_TESTDEV', if_true: files('pc-testdev.c'))
  system_ss.add(when: 'CONFIG_PCA9552', if_true: files('pca9552.c'))
+system_ss.add(when: 'CONFIG_PCA9554', if_true: files('pca9554.c'))
  system_ss.add(when: 'CONFIG_PCI_TESTDEV', if_true: files('pci-testdev.c'))
  system_ss.add(when: 'CONFIG_UNIMP', if_true: files('unimp.c'))
  system_ss.add(when: 'CONFIG_EMPTY_SLOT', if_true: files('empty_slot.c'))
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index f77ca773cf..2302778265 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -33,6 +33,7 @@ config POWERNV
  select FDT_PPC
  select PCI_POWERNV
  select PCA9552
+select PCA9554
  
  config PPC405

  bool
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index c29a136465..54ebef789e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1918,6 +1918,12 @@ static void pnv_rainier_i2c_init(PnvMachineState *pnv)
qdev_get_gpio_in(DEVICE(hotplug), 8));
  qdev_connect_gpio_out(DEVICE(hotplug), 4,
qdev_get_gpio_in(DEVICE(hotplug), 9));
+
+/*
+ * Add a PCA9554 I2C device for cable card presence detection
+ * to engine 2, bus 1, address 0x25
+ */
+i2c_slave_create_simple(chip10->i2c[2].busses[1], "pca9554", 0x25);
  }
  }
  

Re: [PATCH v4 03/11] ppc/pnv: New powernv10-rainier machine type

[Cédric Le Goater]

On 11/21/23 17:36, Miles Glenn wrote:

On Tue, 2023-11-21 at 08:29 +0100, Cédric Le Goater wrote:

On 11/21/23 02:33, Nicholas Piggin wrote:

On Tue Nov 21, 2023 at 9:51 AM AEST, Glenn Miles wrote:

Create a new powernv machine type, powernv10-rainier, that
will contain rainier-specific devices.


Is the plan to have a base powernv10 common to all and then
powernv10-rainier looks like a Rainier? Or would powernv10
just be a rainier?

It's fine to structure code this way, I'm just wondering about
the machine types available to user. Is a base powernv10 machine
useful to run?


There are multiple P10 boards defined in Linux :

aspeed-bmc-ibm-bonnell.dts
aspeed-bmc-ibm-everest.dts
aspeed-bmc-ibm-rainier-1s4u.dts
aspeed-bmc-ibm-rainier-4u.dts
aspeed-bmc-ibm-rainier.dts

and we could model the machines above with a fixed number of sockets.
The "powernv10" would be the generic system that can be customized
at will on the command line, even I2C devices. There is also the
P10 Denali which is FSP based. This QEMU machine would certainly be
very different. I thought of doing the same for P9 with a -zaius
and include NPU2 models for it. I lacked time and the interest was
small at the time of OpenPOWER.

Anyhow, adding a new machine makes sense and it prepares ground for
possible new ones. I am OK with or without. As primary users, you are
the ones that can tell if there will be a second machine.
   
Thanks,


C.



I am not sure what the powernv10 machine would be used for. 


This would be an empty board with only POWER10 processors, no
platform devices.


Thanks,

C.


The
only reason I kept it was because I didn't want to break anyone out
there that might be using it.

My preference would have been to just make powernv10-rainier an
alias of the powernv10 machine, but only one alias name per machine
is supported and there is already a plan to make "powernv" an
alias for the powernv10 machine.

Thanks,

Glenn




Signed-off-by: Glenn Miles 
---
   hw/ppc/pnv.c | 29 +++--
   1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 9c29727337..3481a1220e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2249,7 +2249,7 @@ static void
pnv_machine_power9_class_init(ObjectClass *oc, void *data)
   machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
   }
   
-static void pnv_machine_power10_class_init(ObjectClass *oc, void

*data)
+static void pnv_machine_p10_common_class_init(ObjectClass *oc,
void *data)
   {
   MachineClass *mc = MACHINE_CLASS(oc);
   PnvMachineClass *pmc = PNV_MACHINE_CLASS(oc);
@@ -2261,7 +2261,6 @@ static void
pnv_machine_power10_class_init(ObjectClass *oc, void *data)
   { TYPE_PNV_PHB_ROOT_PORT, "version", "5" },
   };
   
-mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";

   mc->default_cpu_type =
POWERPC_CPU_TYPE_NAME("power10_v2.0");
   compat_props_add(mc->compat_props, phb_compat,
G_N_ELEMENTS(phb_compat));
   
@@ -2274,6 +2273,23 @@ static void

pnv_machine_power10_class_init(ObjectClass *oc, void *data)
   machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
   }
   
+static void pnv_machine_power10_class_init(ObjectClass *oc, void

*data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+pnv_machine_p10_common_class_init(oc, data);
+mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";
+
+}
+
+static void pnv_machine_p10_rainier_class_init(ObjectClass *oc,
void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+pnv_machine_p10_common_class_init(oc, data);
+mc->desc = "IBM PowerNV (Non-Virtualized) POWER10 rainier";
+}
+
   static bool pnv_machine_get_hb(Object *obj, Error **errp)
   {
   PnvMachineState *pnv = PNV_MACHINE(obj);
@@ -2379,6 +2395,15 @@ static void
pnv_machine_class_init(ObjectClass *oc, void *data)
   }
   
   static const TypeInfo types[] = {

+{
+.name  = MACHINE_TYPE_NAME("powernv10-rainier"),
+.parent= TYPE_PNV_MACHINE,
+.class_init= pnv_machine_p10_rainier_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_XIVE_FABRIC },
+{ },
+},
+},
   {
   .name  = MACHINE_TYPE_NAME("powernv10"),
   .parent= TYPE_PNV_MACHINE,

[ANNOUNCE] QEMU 8.1.3 Stable released

[Michael Tokarev]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hi everyone,

The QEMU v8.1.3 stable release is now available.

You can grab the tarball from our download page here:

 https://www.qemu.org/download/#source

v8.1.3 is now tagged in the official qemu.git repository, and the
stable-8.1 branch has been updated accordingly:

 https://gitlab.com/qemu-project/qemu/-/commits/stable-8.1?ref_type=heads

Thank you everyone who has been involved and helped with the stable series!

/mjt

Changelog (stable-8.1-hash master-hash Author Name: Commmit-Subject):

179cc58e00 Michael Tokarev:
Update version for 8.1.3 release
d19b4a4215 52c773ce89 Marc-André Lureau:
hw/mips: LOONGSON3V depends on UNIMP device
b88b9585d8 fc58891d04 Peter Maydell:
target/arm: HVC at EL3 should go to EL3, not EL2
25f55508fc 8011b508cf Matthew Rosato:
s390x/pci: only limit DMA aperture if vfio DMA limit reported
150ebd076e 608bdebb60 Daniel Henrique Barboza:
target/riscv/kvm: support KVM_GET_REG_LIST
221c0e1426 082e9e4a58 Daniel Henrique Barboza:
target/riscv/kvm: improve 'init_multiext_cfg' error msg
100feda604 4d96307c5b Marc-André Lureau:
tracetool: avoid invalid escape in Python string
097c347136 ebc14107f1 Ilya Leoshkevich:
tests/tcg/s390x: Test LAALG with negative cc_src
a16eec9fb2 bea402482a Ilya Leoshkevich:
target/s390x: Fix LAALG not updating cc_src
255422dc75 43fecbe7a5 Ilya Leoshkevich:
tests/tcg/s390x: Test CLC with inaccessible second operand
88b778e4c2 aba2ec341c Ilya Leoshkevich:
target/s390x: Fix CLC corrupting cc_src
07c2a9c0fa cc610857bb Fiona Ebner:
tests/qtest: ahci-test: add test exposing reset issue with pending callback
60f7b60429 7d7512019f Fiona Ebner:
hw/ide: reset: cancel async DMA operation before resetting state
f15258b196 18f86aecd6 Philippe Mathieu-Daudé:
target/mips: Fix TX79 LQ/SQ opcodes
f68a36b7c4 04591b3ddd Philippe Mathieu-Daudé:
target/mips: Fix MSA BZ/BNZ opcodes displacement
801b7e4390 47fd6ab1e3 Dongwon Kim:
ui/gtk-egl: apply scale factor when calculating window's dimension
30d90aebcd 565f85a9c2 Marc-André Lureau:
ui/gtk: force realization of drawing area
e301a77abb 08730ee0cc BALATON Zoltan:
ati-vga: Implement fallback for pixman routines
bcc9879c1c ad4feaca61 Naohiro Aota:
file-posix: fix over-writing of returning zone_append offset
a753815aa8 10b9e0802a Sam Li:
block/file-posix: fix update_zones_wp() caller
cc9f53b3ec b2b109041e Jean-Louis Dupond:
qcow2: keep reference on zeroize with discard-no-unref enabled
fe8eb3187c 5722fc4712 Peter Maydell:
target/arm: Fix A64 LDRA immediate decode
1f560fa276 cc8fb0c3ae Vladimir Sementsov-Ogievskiy:
block/nvme: nvme_process_completion() fix bound for cid
a89c8b96a8 9c549ab689 Marc-André Lureau:
virtio-gpu: block migration of VMs with blob=true
287303495c a1c1082908 David Woodhouse:
hw/xen: use correct default protocol for xen-block on x86
30a4cc2723 debc995e88 David Woodhouse:
hw/xen: take iothread mutex in xen_evtchn_reset_op()
b644416ec6 4a5780f520 David Woodhouse:
hw/xen: fix XenStore watch delivery to guest
0f2dd05b9f 3de75ed352 David Woodhouse:
hw/xen: don't clear map_track[] in xen_gnttab_reset()
5450203719 18e83f28bf David Woodhouse:
hw/xen: select kernel mode for per-vCPU event channel upcall vector
91d789a891 e7dbb62ff1 David Woodhouse:
i386/xen: fix per-vCPU upcall vector for Xen emulation
3321ec125f e969f992c6 David Woodhouse:
i386/xen: Don't advertise XENFEAT_supervisor_mode_kernel
5f0083a95d 4ef9d97b1a Cédric Le Goater:
util/uuid: Remove UUID_FMT_LEN
47c408b80e f8d6f3b16c Cédric Le Goater:
vfio/pci: Fix buffer overrun when writing the VF token
6ede082daf 721da0396c Cédric Le Goater:
util/uuid: Add UUID_STR_LEN definition
155856d890 4c09abeae8 Peter Maydell:
target/arm: Correctly propagate stage 1 BTI guarded bit in a two-stage walk
baf28675da b11293c212 Richard Henderson:
target/arm: Fix SVE STR increment
b7867c8262 827171c318 Andrey Drobyshev:
qemu-iotests: 024: add rebasing test case for overlay_size > backing_size
bd8d9c618a 8b097fd6b0 Andrey Drobyshev:
qemu-img: rebase: stop when reaching EOF of old backing file
de18cbdaf2 580731dcc8 Akihiko Odaki:
tests/tcg: Add -fno-stack-protector
3a2d501916 fed8245015 Kevin Wolf:
block: Fix locking in media change monitor commands
b938418f0d 6f83dc6716 Glenn Miles:
misc/led: LED state is set opposite of what is expected
143352cd7d 307521d6e2 Peter Maydell:
target/arm: Fix syndrome for FGT traps on ERET
fd4ce7455f 930f1865cc Richard Henderson:
target/sparc: Clear may_lookup for npc == DYNAMIC_PC
28facf1598 4ab9a7429b Peter Maydell:
hw/rdma/vmw/pvrdma_cmd: Use correct struct in query_port()
fdeedb886a ae5f70baf5 Lu Gao:
hw/sd/sdhci: Block Size Register bits [14:12] is lost
31c6d1d654 d01448c79d Michal Orzel:
target/arm: Fix CNTPCT_EL0 trapping from EL0 when HCR_EL2.E2H is 0
59ee12a961 a1e6a5c462 Helge Deller:
lasips2: LASI PS/2 devices are not user-createable
1e4c468ec7 3b894b699c Mikulas Patocka:
linux-user/sh4: Fix crashes on signal delivery
814f91d679 6fad9b4bb9 Mikulas Patocka:
linux-user/mips: fix abort on 

[ANNOUNCE] QEMU 7.2.7 Stable released

[Michael Tokarev]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hi everyone,

The QEMU v7.2.7 stable release is now available.

You can grab the tarball from our download page here:

 https://www.qemu.org/download/#source

v7.2.7 is now tagged in the official qemu.git repository, and the
stable-7.2 branch has been updated accordingly:

 https://gitlab.com/qemu-project/qemu/-/commits/stable-7.2?ref_type=heads

Thank you everyone who has been involved and helped with the stable series!

/mjt

Changelog (stable-7.2-hash master-hash Author Name: Commmit-Subject):

14f0c7e3be Michael Tokarev:
Update version for 7.2.7 release
a536d6a486 f8cfdd2038 Bastian Koppelmann:
target/tricore: Rename tricore_feature
38312c2c3b 4d96307c5b Marc-André Lureau:
tracetool: avoid invalid escape in Python string
15764a7635 ebc14107f1 Ilya Leoshkevich:
tests/tcg/s390x: Test LAALG with negative cc_src
154760b6db bea402482a Ilya Leoshkevich:
target/s390x: Fix LAALG not updating cc_src
8e8cae1de8 cc610857bb Fiona Ebner:
tests/qtest: ahci-test: add test exposing reset issue with pending callback
471a9310fd 7d7512019f Fiona Ebner:
hw/ide: reset: cancel async DMA operation before resetting state
18460b3781 18f86aecd6 Philippe Mathieu-Daudé:
target/mips: Fix TX79 LQ/SQ opcodes
9bc1741af1 04591b3ddd Philippe Mathieu-Daudé:
target/mips: Fix MSA BZ/BNZ opcodes displacement
2c1c0cdc25 47fd6ab1e3 Dongwon Kim:
ui/gtk-egl: apply scale factor when calculating window's dimension
ad8457e85e 565f85a9c2 Marc-André Lureau:
ui/gtk: force realization of drawing area
091798aaf0 08730ee0cc BALATON Zoltan:
ati-vga: Implement fallback for pixman routines
0d7c40a1e2 cc8fb0c3ae Vladimir Sementsov-Ogievskiy:
block/nvme: nvme_process_completion() fix bound for cid
26bb3ab8ff 4c09abeae8 Peter Maydell:
target/arm: Correctly propagate stage 1 BTI guarded bit in a two-stage walk
e19b7b8165 fcc0b0418f Peter Maydell:
target/arm: Fix handling of SW and NSW bits for stage 2 walks
6861482dea 21a4ab8318 Peter Maydell:
target/arm: Don't allow stage 2 page table walks to downgrade to NS
d9da3f8dbd 0d3de77a07 Fabiano Rosas:
target/arm: Don't access TCG code when debugging with KVM
3e273f4c16 6003159ce1 Daniel P. Berrangé:
Revert "linux-user: fix compat with glibc >= 2.36 sys/mount.h"
cc64f9ac3d 9f0246539a Daniel P. Berrangé:
Revert "linux-user: add more compat ioctl definitions"
e4a44eb819 827171c318 Andrey Drobyshev:
qemu-iotests: 024: add rebasing test case for overlay_size > backing_size
1e67bd7c21 8b097fd6b0 Andrey Drobyshev:
qemu-img: rebase: stop when reaching EOF of old backing file
6367e823ca 580731dcc8 Akihiko Odaki:
tests/tcg: Add -fno-stack-protector
30811710fb 7a06a8fec9 Akihiko Odaki:
tests/migration: Add -fno-stack-protector
d241c15f75 6f83dc6716 Glenn Miles:
misc/led: LED state is set opposite of what is expected
cec02bc037 ae5f70baf5 Lu Gao:
hw/sd/sdhci: Block Size Register bits [14:12] is lost
ef93ba0d7b a1e6a5c462 Helge Deller:
lasips2: LASI PS/2 devices are not user-createable
cb79e6a451 3b894b699c Mikulas Patocka:
linux-user/sh4: Fix crashes on signal delivery
ea3c95a6b0 6fad9b4bb9 Mikulas Patocka:
linux-user/mips: fix abort on integer overflow
f5358bc18b caea03279e Fabiano Rosas:
migration: Fix analyze-migration read operation signedness
06c9bf032f 85fc35afa9 Yuval Shaia:
hw/pvrdma: Protect against buggy or malicious guest driver
0c049eafd5 cffa995490 Alvin Chang:
disas/riscv: Fix the typo of inverted order of pmpaddr13 and pmpaddr14
c19fd37eb3 00e3b29d06 Volker Rümelin:
hw/audio/es1370: reset current sample counter
77d36ba300 86dec715a7 Peter Xu:
migration/qmp: Fix crash on setting tls-authz with null
85c27e5b37 0114c45130 Akihiko Odaki:
amd_iommu: Fix APIC address check
24d5b71884 33bc4fa78b Richard Henderson:
linux-user/hppa: Fix struct target_sigcontext layout
5f2b5606e2 4f7689f081 Thomas Huth:
chardev/char-pty: Avoid losing bytes when the other side just (re-)connected
e12abe5a7d e0288a7784 Laszlo Ersek:
hw/display/ramfb: plug slight guest-triggerable leak on mode setting
755cf29ead abd41884c5 Paolo Bonzini:
target/i386: fix memory operand size for CVTPS2PD
796468c24a a48b26978a Paolo Bonzini:
target/i386: generalize operand size "ph" for use in CVTPS2PD
327d65ea97 8bf171c2d1 Ricky Zhou:
target/i386: Fix exception classes for MOVNTPS/MOVNTPD.
72ef83b12f cab529b0dc Ricky Zhou:
target/i386: Fix exception classes for SSE/AVX instructions.
cebd957e7b afa94dabc5 Ricky Zhou:
target/i386: Fix and add some comments next to SSE/AVX instructions.
91fa137979 9e65829699 Paolo Bonzini:
tests/tcg/i386: correct mask for VPERM2F128/VPERM2I128
c9a2d122bf 3d304620ec Paolo Bonzini:
target/i386: fix operand size of unary SSE operations
8da0da8799 be2b619a17 Mark Cave-Ayland:
scsi-disk: ensure that FORMAT UNIT commands are terminated
fa2f090d62 77668e4b9b Mark Cave-Ayland:
esp: restrict non-DMA transfer length to that of available data
9b7feb87d0 b86dc5cb0b Mark Cave-Ayland:
esp: use correct type for esp_dma_enable() in sysbus_esp_gpio_demux()
ae0b40d9d9 35ed01ba54 Fabiano Rosas:

Re: [PATCH v4 03/11] ppc/pnv: New powernv10-rainier machine type

[Miles Glenn]

On Tue, 2023-11-21 at 07:46 +0100, Cédric Le Goater wrote:
> On 11/21/23 00:51, Glenn Miles wrote:
> > Create a new powernv machine type, powernv10-rainier, that
> > will contain rainier-specific devices.
> > 
> > Signed-off-by: Glenn Miles 
> > ---
> >   hw/ppc/pnv.c | 29 +++--
> >   1 file changed, 27 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 9c29727337..3481a1220e 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -2249,7 +2249,7 @@ static void
> > pnv_machine_power9_class_init(ObjectClass *oc, void *data)
> >   machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
> >   }
> >   
> > -static void pnv_machine_power10_class_init(ObjectClass *oc, void
> > *data)
> > +static void pnv_machine_p10_common_class_init(ObjectClass *oc,
> > void *data)
> >   {
> >   MachineClass *mc = MACHINE_CLASS(oc);
> >   PnvMachineClass *pmc = PNV_MACHINE_CLASS(oc);
> > @@ -2261,7 +2261,6 @@ static void
> > pnv_machine_power10_class_init(ObjectClass *oc, void *data)
> >   { TYPE_PNV_PHB_ROOT_PORT, "version", "5" },
> >   };
> >   
> > -mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";
> 
> I would keep this description because the "powernv10" machine still
> can
> be used, without I2C devices. Unless we don't want to ?
> 

I'm not sure about the usefulness of the powernv10 machine, but the
description still exists in the pnv_machine_power10_class_init function
(and is unchanged).  The pnv_machine_p10_common_class_init function was
created to hold initialization of things that are common between all
P10 machines, and is called by all power10 based machines (powernv10
and powernv10-rainier so far).

> >   mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power10_v2.0");
> >   compat_props_add(mc->compat_props, phb_compat,
> > G_N_ELEMENTS(phb_compat));
> >   
> > @@ -2274,6 +2273,23 @@ static void
> > pnv_machine_power10_class_init(ObjectClass *oc, void *data)
> >   machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PNV_PHB);
> >   }
> >   
> > +static void pnv_machine_power10_class_init(ObjectClass *oc, void
> > *data)
> > +{
> > +MachineClass *mc = MACHINE_CLASS(oc);
> > +
> > +pnv_machine_p10_common_class_init(oc, data);
> > +mc->desc = "IBM PowerNV (Non-Virtualized) POWER10";
> > +
> 
> Superfluous white line.
> 
Removed in v5.

> > +}
> > +
> > +static void pnv_machine_p10_rainier_class_init(ObjectClass *oc,
> > void *data)
> > +{
> > +MachineClass *mc = MACHINE_CLASS(oc);
> > +
> > +pnv_machine_p10_common_class_init(oc, data);
> > +mc->desc = "IBM PowerNV (Non-Virtualized) POWER10 rainier";
> > +}
> > +
> >   static bool pnv_machine_get_hb(Object *obj, Error **errp)
> >   {
> >   PnvMachineState *pnv = PNV_MACHINE(obj);
> > @@ -2379,6 +2395,15 @@ static void
> > pnv_machine_class_init(ObjectClass *oc, void *data)
> >   }
> >   
> >   static const TypeInfo types[] = {
> > +{
> > +.name  = MACHINE_TYPE_NAME("powernv10-rainier"),
> > +.parent= TYPE_PNV_MACHINE,
> 
> Could the parent be :
> 
>  .parent  = MACHINE_TYPE_NAME("powernv10"),
> 
> This should avoid the definition of the .interfaces below.
> 
> Thanks,
> 
> C.
> 

Changed as suggested in v5.

Thanks,

Glenn

> 
> 
> > +.class_init= pnv_machine_p10_rainier_class_init,
> > +.interfaces = (InterfaceInfo[]) {
> > +{ TYPE_XIVE_FABRIC },
> > +{ },
> > +},
> > +},
> >   {
> >   .name  = MACHINE_TYPE_NAME("powernv10"),
> >   .parent= TYPE_PNV_MACHINE,

[PATCH-for-8.2? 3/6] hw/misc/mps2-scc: Free MPS2SCC::oscclk[] array on finalize()

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit 4fb013afcc added:

  DEFINE_PROP_ARRAY("oscclk", MPS2SCC, num_oscclk, oscclk_reset,
qdev_prop_uint32, uint32_t),

but forgot to free the 'oscclk_reset' array. Do it in the
instance_finalize() handler.

Cc: qemu-sta...@nongnu.org
Fixes: 4fb013afcc ("hw/misc/mps2-scc: Support configurable number of OSCCLK 
values") # v6.0.0+
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/misc/mps2-scc.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
index b3b42a792c..fe5034db14 100644
--- a/hw/misc/mps2-scc.c
+++ b/hw/misc/mps2-scc.c
@@ -329,6 +329,13 @@ static void mps2_scc_realize(DeviceState *dev, Error 
**errp)
 s->oscclk = g_new0(uint32_t, s->num_oscclk);
 }
 
+static void mps2_scc_finalize(Object *obj)
+{
+MPS2SCC *s = MPS2_SCC(obj);
+
+g_free(s->oscclk_reset);
+}
+
 static const VMStateDescription mps2_scc_vmstate = {
 .name = "mps2-scc",
 .version_id = 3,
@@ -385,6 +392,7 @@ static const TypeInfo mps2_scc_info = {
 .parent = TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(MPS2SCC),
 .instance_init = mps2_scc_init,
+.instance_finalize = mps2_scc_finalize,
 .class_init = mps2_scc_class_init,
 };
 
-- 
2.41.0

[PATCH-for-8.2 6/6] hw/input/stellaris_gamepad: Free StellarisGamepad::keycodes[] array

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit a75f336b97 added:

  DEFINE_PROP_ARRAY("keycodes", StellarisGamepad, num_buttons,
keycodes, qdev_prop_uint32, uint32_t),

but forgot to free the 'keycodes' array. Do it in the instance_finalize
handler.

Fixes: a75f336b97 ("hw/input/stellaris_input: Convert to qdev")
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/input/stellaris_gamepad.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/input/stellaris_gamepad.c b/hw/input/stellaris_gamepad.c
index 06a0c0ce83..9dfa620e29 100644
--- a/hw/input/stellaris_gamepad.c
+++ b/hw/input/stellaris_gamepad.c
@@ -63,6 +63,13 @@ static void stellaris_gamepad_realize(DeviceState *dev, 
Error **errp)
 qemu_input_handler_register(dev, _gamepad_handler);
 }
 
+static void stellaris_gamepad_finalize(Object *obj)
+{
+StellarisGamepad *s = STELLARIS_GAMEPAD(obj);
+
+g_free(s->keycodes);
+}
+
 static void stellaris_gamepad_reset_enter(Object *obj, ResetType type)
 {
 StellarisGamepad *s = STELLARIS_GAMEPAD(obj);
@@ -92,6 +99,7 @@ static const TypeInfo stellaris_gamepad_info[] = {
 .name = TYPE_STELLARIS_GAMEPAD,
 .parent = TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(StellarisGamepad),
+.instance_finalize = stellaris_gamepad_finalize,
 .class_init = stellaris_gamepad_class_init,
 },
 };
-- 
2.41.0

[PATCH-for-8.2? 4/6] hw/nvram/xlnx-efuse: Free XlnxEFuse::ro_bits[] array on finalize()

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit 68fbcc344e added:

  DEFINE_PROP_ARRAY("read-only", XlnxEFuse, ro_bits_cnt, ro_bits,
qdev_prop_uint32, uint32_t),

but forgot to free the 'ro_bits' array. Do it in the instance_finalize
handler.

Cc: qemu-sta...@nongnu.org
Fixes: 68fbcc344e ("hw/nvram: Introduce Xilinx eFuse QOM") # v6.2.0+
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/nvram/xlnx-efuse.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
index 655c40b8d1..f7b849f7de 100644
--- a/hw/nvram/xlnx-efuse.c
+++ b/hw/nvram/xlnx-efuse.c
@@ -224,6 +224,13 @@ static void efuse_realize(DeviceState *dev, Error **errp)
 }
 }
 
+static void efuse_finalize(Object *obj)
+{
+XlnxEFuse *s = XLNX_EFUSE(obj);
+
+g_free(s->ro_bits);
+}
+
 static void efuse_prop_set_drive(Object *obj, Visitor *v, const char *name,
  void *opaque, Error **errp)
 {
@@ -280,6 +287,7 @@ static const TypeInfo efuse_info = {
 .name  = TYPE_XLNX_EFUSE,
 .parent= TYPE_DEVICE,
 .instance_size = sizeof(XlnxEFuse),
+.instance_finalize = efuse_finalize,
 .class_init= efuse_class_init,
 };
 
-- 
2.41.0

[PATCH-for-8.2? 5/6] hw/nvram/xlnx-efuse-ctrl: Free XlnxVersalEFuseCtrl[] "pg0-lock" array

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit 9e4aa1fafe added:

  DEFINE_PROP_ARRAY("pg0-lock",
XlnxVersalEFuseCtrl, extra_pg0_lock_n16,
extra_pg0_lock_spec, qdev_prop_uint16, uint16_t),

but forgot to free the 'extra_pg0_lock_spec' array. Do it in the
instance_finalize() handler.

Cc: qemu-sta...@nongnu.org
Fixes: 9e4aa1fafe ("hw/nvram: Xilinx Versal eFuse device") # v6.2.0+
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/nvram/xlnx-versal-efuse-ctrl.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/nvram/xlnx-versal-efuse-ctrl.c 
b/hw/nvram/xlnx-versal-efuse-ctrl.c
index beb5661c35..2480af35e1 100644
--- a/hw/nvram/xlnx-versal-efuse-ctrl.c
+++ b/hw/nvram/xlnx-versal-efuse-ctrl.c
@@ -726,6 +726,13 @@ static void efuse_ctrl_init(Object *obj)
 sysbus_init_irq(sbd, >irq_efuse_imr);
 }
 
+static void efuse_ctrl_finalize(Object *obj)
+{
+XlnxVersalEFuseCtrl *s = XLNX_VERSAL_EFUSE_CTRL(obj);
+
+g_free(s->extra_pg0_lock_spec);
+}
+
 static const VMStateDescription vmstate_efuse_ctrl = {
 .name = TYPE_XLNX_VERSAL_EFUSE_CTRL,
 .version_id = 1,
@@ -764,6 +771,7 @@ static const TypeInfo efuse_ctrl_info = {
 .instance_size = sizeof(XlnxVersalEFuseCtrl),
 .class_init= efuse_ctrl_class_init,
 .instance_init = efuse_ctrl_init,
+.instance_finalize = efuse_ctrl_finalize,
 };
 
 static void efuse_ctrl_register_types(void)
-- 
2.41.0

[PATCH-for-8.2? 1/6] hw/virtio: Add VirtioPCIDeviceTypeInfo::instance_finalize field

[Philippe Mathieu-Daudé]

The VirtioPCIDeviceTypeInfo structure, added in commit a4ee4c8baa
("virtio: Helper for registering virtio device types") got extended
in commit 8ea90ee690 ("virtio: add class_size") with the @class_size
field. Do similarly with the @instance_finalize field.

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

diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 5a3f182f99..59d88018c1 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -246,6 +246,7 @@ typedef struct VirtioPCIDeviceTypeInfo {
 size_t instance_size;
 size_t class_size;
 void (*instance_init)(Object *obj);
+void (*instance_finalize)(Object *obj);
 void (*class_init)(ObjectClass *klass, void *data);
 InterfaceInfo *interfaces;
 } VirtioPCIDeviceTypeInfo;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 205dbf24fb..e433879542 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2391,6 +2391,7 @@ void virtio_pci_types_register(const 
VirtioPCIDeviceTypeInfo *t)
 .parent= t->parent ? t->parent : TYPE_VIRTIO_PCI,
 .instance_size = t->instance_size,
 .instance_init = t->instance_init,
+.instance_finalize = t->instance_finalize,
 .class_size= t->class_size,
 .abstract  = true,
 .interfaces= t->interfaces,
-- 
2.41.0

[PATCH-for-8.2 0/6] hw: Free DEFINE_PROP_ARRAY()'s arrays in instance_finalize()

[Philippe Mathieu-Daudé]

In few places we forget to free the array allocated by the
DEFINE_PROP_ARRAY() macro handlers. Fix that.

Philippe Mathieu-Daudé (6):
  hw/virtio: Add VirtioPCIDeviceTypeInfo::instance_finalize field
  hw/virtio: Free VirtIOIOMMUPCI::vdev.reserved_regions[] on finalize()
  hw/misc/mps2-scc: Free MPS2SCC::oscclk[] array on finalize()
  hw/nvram/xlnx-efuse: Free XlnxEFuse::ro_bits[] array on finalize()
  hw/nvram/xlnx-efuse-ctrl: Free XlnxVersalEFuseCtrl[] "pg0-lock" array
  hw/input/stellaris_gamepad: Free StellarisGamepad::keycodes[] array

 include/hw/virtio/virtio-pci.h| 1 +
 hw/input/stellaris_gamepad.c  | 8 
 hw/misc/mps2-scc.c| 8 
 hw/nvram/xlnx-efuse.c | 8 
 hw/nvram/xlnx-versal-efuse-ctrl.c | 8 
 hw/virtio/virtio-iommu-pci.c  | 8 
 hw/virtio/virtio-pci.c| 1 +
 7 files changed, 42 insertions(+)

-- 
2.41.0

[PATCH-for-8.2? 2/6] hw/virtio: Free VirtIOIOMMUPCI::vdev.reserved_regions[] on finalize()

[Philippe Mathieu-Daudé]

Commit 0be6bfac62 ("qdev: Implement variable length array properties")
added the DEFINE_PROP_ARRAY() macro with the following comment:

  * It is the responsibility of the device deinit code to free the
  * @_arrayfield memory.

Commit 8077b8e549 added:

  DEFINE_PROP_ARRAY("reserved-regions", VirtIOIOMMUPCI,
vdev.nb_reserved_regions, vdev.reserved_regions,
qdev_prop_reserved_region, ReservedRegion),

but forgot to free the 'vdev.reserved_regions' array. Do it in the
instance_finalize() handler.

Cc: qemu-sta...@nongnu.org
Fixes: 8077b8e549 ("virtio-iommu-pci: Add array of Interval properties") # 
v5.1.0+
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/virtio/virtio-iommu-pci.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 9459fbf6ed..cbdfe4c591 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -95,10 +95,18 @@ static void virtio_iommu_pci_instance_init(Object *obj)
 TYPE_VIRTIO_IOMMU);
 }
 
+static void virtio_iommu_pci_instance_finalize(Object *obj)
+{
+VirtIOIOMMUPCI *dev = VIRTIO_IOMMU_PCI(obj);
+
+g_free(dev->vdev.prop_resv_regions);
+}
+
 static const VirtioPCIDeviceTypeInfo virtio_iommu_pci_info = {
 .generic_name  = TYPE_VIRTIO_IOMMU_PCI,
 .instance_size = sizeof(VirtIOIOMMUPCI),
 .instance_init = virtio_iommu_pci_instance_init,
+.instance_finalize = virtio_iommu_pci_instance_finalize,
 .class_init= virtio_iommu_pci_class_init,
 };
 
-- 
2.41.0

[PATCH for-8.2 0/2] qdev array property fixes

[Kevin Wolf]

Kevin Wolf (2):
  qdev: Fix crash in array property getter
  string-output-visitor: Support lists for non-integer types

 hw/core/qdev-properties.c| 33 ++---
 qapi/string-output-visitor.c | 24 
 2 files changed, 46 insertions(+), 11 deletions(-)

-- 
2.42.0

[PATCH for-8.2 1/2] qdev: Fix crash in array property getter

[Kevin Wolf]

Passing an uninitialised list to visit_start_list() happens to work for
the QObject output visitor because it treats the pointer as an opaque
value and never dereferences it, but the string output visitor expects a
valid list to check if it has more than one element.

The existing code crashes with the string output visitor if the
uninitialised value is non-NULL. Passing an explicit NULL would fix the
crash, but still result in wrong output.

Rework get_prop_array() so that it conforms to the expectations that the
string output visitor has. This includes building a real list first and
using visit_next_list() to iterate it.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1993
Reported-by: Thomas Huth 
Signed-off-by: Kevin Wolf 
---
 hw/core/qdev-properties.c | 33 ++---
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 91632f7be9..840006e953 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -689,23 +689,36 @@ static void get_prop_array(Object *obj, Visitor *v, const 
char *name,
 Property *prop = opaque;
 uint32_t *alenptr = object_field_prop_ptr(obj, prop);
 void **arrayptr = (void *)obj + prop->arrayoffset;
-char *elem = *arrayptr;
-GenericList *list;
-const size_t list_elem_size = sizeof(*list) + prop->arrayfieldsize;
+char *elemptr = *arrayptr;
+ArrayElementList *list = NULL, *elem;
+ArrayElementList **tail = 
+const size_t size = sizeof(*list);
 int i;
 bool ok;
 
-if (!visit_start_list(v, name, , list_elem_size, errp)) {
+/* At least the string output visitor needs a real list */
+for (i = 0; i < *alenptr; i++) {
+elem = g_new0(ArrayElementList, 1);
+elem->value = elemptr;
+elemptr += prop->arrayfieldsize;
+
+*tail = elem;
+tail = >next;
+}
+
+if (!visit_start_list(v, name, (GenericList **) , size, errp)) {
 return;
 }
 
-for (i = 0; i < *alenptr; i++) {
-Property elem_prop = array_elem_prop(obj, prop, name, elem);
+elem = list;
+while (elem) {
+Property elem_prop = array_elem_prop(obj, prop, name, elem->value);
 prop->arrayinfo->get(obj, v, NULL, _prop, errp);
 if (*errp) {
 goto out_obj;
 }
-elem += prop->arrayfieldsize;
+elem = (ArrayElementList *) visit_next_list(v, (GenericList*) elem,
+size);
 }
 
 /* visit_check_list() can only fail for input visitors */
@@ -714,6 +727,12 @@ static void get_prop_array(Object *obj, Visitor *v, const 
char *name,
 
 out_obj:
 visit_end_list(v, (void**) );
+
+while (list) {
+elem = list;
+list = elem->next;
+g_free(elem);
+}
 }
 
 static void default_prop_array(ObjectProperty *op, const Property *prop)
-- 
2.42.0

[PATCH for-8.2 2/2] string-output-visitor: Support lists for non-integer types

[Kevin Wolf]

With the introduction of list-based array properties in qdev, the string
output visitor has to deal with lists of non-integer elements now ('info
qtree' prints all properties with the string output visitor).

Currently there is no explicit support for such lists, and the resulting
output is only the last element because string_output_set() always
replaces the output with the latest value. Instead of replacing the old
value, append comma separated values in list context.

The difference can be observed in 'info qtree' with a 'rocker' device
that has a 'ports' list with more than one element.

Signed-off-by: Kevin Wolf 
---
 qapi/string-output-visitor.c | 24 
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 71ddc92b7b..c0cb72dbe4 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -74,11 +74,27 @@ static StringOutputVisitor *to_sov(Visitor *v)
 
 static void string_output_set(StringOutputVisitor *sov, char *string)
 {
-if (sov->string) {
-g_string_free(sov->string, true);
+switch (sov->list_mode) {
+case LM_STARTED:
+sov->list_mode = LM_IN_PROGRESS;
+/* fall through */
+case LM_NONE:
+if (sov->string) {
+g_string_free(sov->string, true);
+}
+sov->string = g_string_new(string);
+g_free(string);
+break;
+
+case LM_IN_PROGRESS:
+case LM_END:
+g_string_append(sov->string, ", ");
+g_string_append(sov->string, string);
+break;
+
+default:
+abort();
 }
-sov->string = g_string_new(string);
-g_free(string);
 }
 
 static void string_output_append(StringOutputVisitor *sov, int64_t a)
-- 
2.42.0

Re: [PATCH] scripts: adjust url to Coverity tools

[Peter Maydell]

On Tue, 21 Nov 2023 at 17:05, Paolo Bonzini  wrote:
>
> The URL to the Coverity tools download has changed; the old one points
> to an obsolete version that is not supported anymore.  Adjust to point
> to the correct and supported tools.
>
> Suggested-by: Peter Maydell 
> Signed-off-by: Paolo Bonzini 
> ---
>  scripts/coverity-scan/run-coverity-scan | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: Instruction virtual address in TCG Plugins

[Alex Bennée]

Mikhail Tyutin  writes:

>> >> > I suspect it is because of memory mappings by guest OS that changes 
>> >> > virtual addresses for that block.
>> >> >
>> >> > I also looked at gen_empty_udata_cb() function and considered to extend 
>> >> > plugin API to pass a program counter
>> >> > value as additional callback argument. I thought it would always give 
>> >> > me valid virtual address of an instruction.
>> >> > Unfortunately, I didn't find a way to get value of that register in 
>> >> > architecture agnostic way (it is 'pc' member in
>> >> > CPUArchState structure).
>> >>
>> >> When we merge the register api you should be able to do that. Although
>> >> during testing I realised that PC acted funny compared to everything
>> >> else because we don't actually update the shadow register every
>> >> instruction.
>> >
>> > We implemented similar API to read registers (by coincidence, I posted 
>> > this patch at the same time as the API you
>> > mentioned) and I observe similar behavior. As far as I see, CPU state is 
>> > only updated in between of executed translation
>> > blocks. Switching to 'singlestep' mode helps to fix that, but execution 
>> > overhead is huge.
>> >
>> > There is also blocks 'chaining' mechanism which is likely contributes to 
>> > corrupted blocks vaddr inside of callbacks.
>> > My guess is that 'pc' value for those chained blocks points to the first 
>> > block of entire chain. Unfortunately, It is very
>> > hard to debug, because I can only see block chains when I run whole Linux 
>> > guest OS. Does Qemu has small test
>> > application to trigger long enough chain of translation blocks?
>> 
>> No all registers should be resolved by the end of any block. There is
>> currently no optimisation of register usage between TBs. If you are
>> seeing PC corruption that would be a bug - but fundamentally things
>> would break pretty quick if tb_lookup() and friends didn't have an
>> accurate PC.
>
> I managed to root cause source of corrupted addresses in plugin callbacks.
> There were basically 2 problems:
>
> 1. Memory IO operations force TCG to create special translation blocks to
> process that memory load/store operation. The plugin gets notification for
> this translation block as well, but instrumentation callbacks other than
> memory ones are silently ignored. To make it correct, the plugin has to match
> instruction execution callback from previous TB to memory callback from that
> special TB. The fix was to expose internal ‘memOnly’ TB flag to the plugin to
> handle such TBs differently.

Are you talking about the CF_MEMI_ONLY compile flag? We added this to
avoid double counting executed instructions. Has there been a clash with
the other changes to always cpu_recompile_io? This was a change added to
fix: https://gitlab.com/qemu-project/qemu/-/issues/1866

Richard is going to look at optimising the cpu_recompile_io code so we
"lock in" a shortened translation once we discover a block is doing
MMIO. See https://linaro.atlassian.net/browse/QEMU-605 for an overview.

> 2. Another problem is related to interrupts handling. Since we can insert pre-
> callback on instructions only, the plugin is not aware if instruction is
> actually executed or interrupted by an interrupt or exception. In fact, it
> mistakenly interprets all interrupted instructions as executed. Adding API
> to receive interrupt notification and appropriate handling of it fixes
> the problem.

We don't process any interrupts until the start of each block so no
asynchronous IRQs should interrupt execution. However it is possible
that any given instruction could generate a synchronous exception so if
you need a precise count of execution you need to instrument every
single instruction. With enough knowledge the plugin could avoid
instrumenting stuff that will never fault but that relies on baking
additional knowledge into the plugin.

Generally its only memory operations that can fault (although I guess
FPU and some more esoteric integer ops can).

>
> I will send those patches for review shortly and thank you for dissuading me
> from going to wrong direction!

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v7 00/27] vfio: Adopt iommufd

[Cédric Le Goater]

Hello Zhenzhong

On 11/21/23 09:43, Zhenzhong Duan wrote:

Hi,

Thanks all for giving guides and comments on previous series, this is
the remaining part of the iommufd support.

Besides suggested changes in v6, I'd like to highlight two changes
for final review:
1. Instantiate can_be_deleted callback to fix race where iommufd object
can be deleted before vfio device
2. After careful re-thinking, I'd like to follow Nicolin's suggestion in v5,
remove is_ioas check which indeed looks heavy just for tracepoint.
In fact we can get corresponding info by looking over trace context.

PATCH 1: Introduce iommufd object
PATCH 2-9: add IOMMUFD container and cdev support
PATCH 10-17: fd passing for cdev and linking to IOMMUFD
PATCH 18: make VFIOContainerBase parameter const
PATCH 19-21: Compile out for IOMMUFD for arm, s390x and x86
PATCH 22-26: vfio device init code cleanup
PATCH 27: add iommufd doc


We have done wide test with different combinations, e.g:
- PCI device were tested
- FD passing and hot reset with some trick.
- device hotplug test with legacy and iommufd backends
- with or without vIOMMU for legacy and iommufd backends
- divices linked to different iommufds
- VFIO migration with a E800 net card(no dirty sync support) passthrough
- platform, ccw and ap were only compile-tested due to environment limit
- test mdev pass through with mtty and mix with real device and different BE
- test iommufd object hotplug/unplug and mix with vfio device plug/unplug

Given some iommufd kernel limitations, the iommufd backend is
not yet fully on par with the legacy backend w.r.t. features like:
- p2p mappings (you will see related error traces)
- dirty page sync
- and etc.


qemu code: https://github.com/yiliu1765/qemu/commits/zhenzhong/iommufd_cdev_v7
Based on vfio-next, commit id: c487fb8a50


The series is pushed on top of vfio-next in the vfio-8.2 tree :

  https://github.com/legoater/qemu/commits/vfio-8.2

with a little extra to deal with a PPC build failure.

Thanks,

C.

Re: QEMU Summit Minutes 2023

[Alex Bennée]

Peter Maydell  writes:

> QEMU Summit Minutes 2023
> 
>
> As usual, we held a QEMU Summit meeting at KVM Forum.  This is an
> invite-only meeting for the most active maintainers and submaintainers
> in the project, and we discuss various project-wide issues, usually
> process stuff. We then post the minutes of the meeting to the list as
> a jumping off point for wider discussion and for those who weren't
> able to attend.
>

>
> Topic 2: Are we happy with the email workflow?
> ==
>
> This was a topic to see if there was any consensus among maintainers
> about the long-term acceptability of sticking with email for patch
> submission and review -- in five years' time, if we're still doing it
> the same way, how would we feel about it?
>
> One area where we did get consensus was that now that we're doing CI
> on gitlab we can change pull requests from maintainers from via-email
> to gitlab merge requests. This would hopefully mean that instead of
> the release-manager having to tell gitlab to do a merge and then
> reporting back the results of any CI failures, the maintainer
> could directly see the CI results and deal with fixing up failures
> and resubmitting without involving the release manager. (This
> may have the disbenefit that there isn't a single person any
> more who looks at all the CI results and gets a sense of whether
> particular test cases have pre-existing intermittent failures.)

If we are keen to start processing merge requests for the 9.0 release we
really should consider how it is going to work before we open up the
taps post 8.2-final going out.

Does anyone want to have a go at writing an updated process for
docs/devel/submitting-a-pull-request.rst (or I guess merge-request) so
we can discuss it and be ready early in the cycle? Ideally someone who
already has experience with the workflow with other gitlab hosted
projects.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH] scripts: adjust url to Coverity tools

[Paolo Bonzini]

The URL to the Coverity tools download has changed; the old one points
to an obsolete version that is not supported anymore.  Adjust to point
to the correct and supported tools.

Suggested-by: Peter Maydell 
Signed-off-by: Paolo Bonzini 
---
 scripts/coverity-scan/run-coverity-scan | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/coverity-scan/run-coverity-scan 
b/scripts/coverity-scan/run-coverity-scan
index 129672c86f..d56c9b6677 100755
--- a/scripts/coverity-scan/run-coverity-scan
+++ b/scripts/coverity-scan/run-coverity-scan
@@ -116,14 +116,14 @@ update_coverity_tools () {
 cd "$COVERITY_TOOL_BASE"
 
 echo "Checking for new version of coverity build tools..."
-wget https://scan.coverity.com/download/linux64 --post-data 
"token=$COVERITY_TOKEN=$PROJNAME=1" -O coverity_tool.md5.new
+wget https://scan.coverity.com/download/cxx/linux64 --post-data 
"token=$COVERITY_TOKEN=$PROJNAME=1" -O coverity_tool.md5.new
 
 if ! cmp -s coverity_tool.md5 coverity_tool.md5.new; then
 # out of date md5 or no md5: download new build tool
 # blow away the old build tool
 echo "Downloading coverity build tools..."
 rm -rf coverity_tool coverity_tool.tgz
-wget https://scan.coverity.com/download/linux64 --post-data 
"token=$COVERITY_TOKEN=$PROJNAME" -O coverity_tool.tgz
+wget https://scan.coverity.com/download/cxx/linux64 --post-data 
"token=$COVERITY_TOKEN=$PROJNAME" -O coverity_tool.tgz
 if ! (cat coverity_tool.md5.new; echo "  coverity_tool.tgz") | md5sum 
-c --status; then
 echo "Downloaded tarball didn't match md5sum!"
 exit 1
-- 
2.39.1