Re: [PATCH v2 3/3] hw/{misc, riscv}: pfsoc: add system controller as unimplemented
Hi Conor,
On 12/11/22 14:34, Conor Dooley wrote:
From: Conor Dooley
The system controller on PolarFire SoC is access via a mailbox. The
control registers for this mailbox lie in the "IOSCB" region & the
interrupt is cleared via write to the "SYSREG" region. It also has a
QSPI controller, usually connected to a flash chip, that is used for
storing FPGA bitstreams and used for In-Application Programming (IAP).
Linux has an implementation of the system controller, through which the
hwrng is accessed, leading to load/store access faults.
Add the QSPI as unimplemented and a very basic (effectively
unimplemented) version of the system controller's mailbox. Rather than
purely marking the regions as unimplemented, service the mailbox
requests by reporting failures and raising the interrupt so a guest can
better handle the lack of support.
Signed-off-by: Conor Dooley
---
hw/misc/mchp_pfsoc_ioscb.c | 59 -
hw/misc/mchp_pfsoc_sysreg.c | 19 --
hw/riscv/microchip_pfsoc.c | 6 +++
include/hw/misc/mchp_pfsoc_ioscb.h | 3 ++
include/hw/misc/mchp_pfsoc_sysreg.h | 1 +
include/hw/riscv/microchip_pfsoc.h | 1 +
6 files changed, 83 insertions(+), 6 deletions(-)
@@ -52,10 +54,18 @@ static uint64_t mchp_pfsoc_sysreg_read(void *opaque, hwaddr
offset,
static void mchp_pfsoc_sysreg_write(void *opaque, hwaddr offset,
uint64_t value, unsigned size)
{
-qemu_log_mask(LOG_UNIMP, "%s: unimplemented device write "
- "(size %d, value 0x%" PRIx64
- ", offset 0x%" HWADDR_PRIx ")\n",
- __func__, size, value, offset);
+MchpPfSoCSysregState *s = opaque;
+qemu_irq_lower(s->irq);
Is this always lowered IRQ line wanted? ...
+switch (offset) {
+case MESSAGE_INT:
+qemu_irq_lower(s->irq);
... since we do it here.
+break;
+default:
+qemu_log_mask(LOG_UNIMP, "%s: unimplemented device write "
+ "(size %d, value 0x%" PRIx64
+ ", offset 0x%" HWADDR_PRIx ")\n",
+ __func__, size, value, offset);
+}
}
Re: [PATCH] qga: Add initial OpenBSD and NetBSD support
On 12/11/22 12:40, Brad Smith wrote: qga: Add initial OpenBSD and NetBSD support Signed-off-by: Brad Smith --- meson.build | 2 +- qga/commands-bsd.c | 5 + qga/commands-posix.c | 9 +++-- qga/main.c | 6 +++--- 4 files changed, 16 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v5 09/20] hw/arm: remove current_cpu hack from pxa2xx access
On 11/11/22 19:25, Alex Bennée wrote: We can derive the correct CPU from CPUARMState so lets not rely on current_cpu. Signed-off-by: Alex Bennée --- hw/arm/pxa2xx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v5 10/20] target/microblaze: initialise MemTxAttrs for CPU access
On 11/11/22 19:25, Alex Bennée wrote: Both of these functions deal with CPU based access (as is evidenced by the secure check straight after). Use the new MEMTXATTRS_CPU constructor to ensure the correct CPU id is filled in should it ever be needed by any devices later. Signed-off-by: Alex Bennée --- target/microblaze/helper.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v5 11/20] target/sparc: initialise MemTxAttrs for CPU access
On 11/11/22 19:25, Alex Bennée wrote: Both of the TLB fill functions and the cpu_sparc_get_phys_page deal with CPU based access. Use the new MEMTXATTRS_CPU constructor to ensure the correct CPU id is filled in should it ever be needed by any devices later. Signed-off-by: Alex Bennée --- target/sparc/mmu_helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v5 14/20] hw/audio: explicitly set .requester_type for intel-hda
On 12/11/22 06:50, Richard Henderson wrote:
On 11/12/22 04:25, Alex Bennée wrote:
This is simulating a bus master writing data back into system memory.
Mark it as such.
Signed-off-by: Alex Bennée
---
hw/audio/intel-hda.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index f38117057b..95c28b315c 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -345,7 +345,7 @@ static void intel_hda_corb_run(IntelHDAState *d)
static void intel_hda_response(HDACodecDevice *dev, bool solicited,
uint32_t response)
{
- const MemTxAttrs attrs = { .memory = true };
+ const MemTxAttrs attrs = { .requester_type = MTRT_PCI, .memory =
true };
MEMTXATTRS_PCI?
Then removing the 'const' qualifier and setting .memory after.
Re: [PATCH v5 15/20] hw/i386: update vapic_write to use MemTxAttrs
On 11/11/22 19:25, Alex Bennée wrote:
This allows us to drop the current_cpu hack and properly model an
invalid access to the vapic.
Signed-off-by: Alex Bennée
---
hw/i386/kvmvapic.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 43f8a8f679..a76ed07199 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -635,20 +635,21 @@ static int vapic_prepare(VAPICROMState *s)
return 0;
}
-static void vapic_write(void *opaque, hwaddr addr, uint64_t data,
-unsigned int size)
+static MemTxResult vapic_write(void *opaque, hwaddr addr, uint64_t data,
+ unsigned int size, MemTxAttrs attrs)
{
VAPICROMState *s = opaque;
+CPUState *cs;
X86CPU *cpu;
CPUX86State *env;
hwaddr rom_paddr;
-if (!current_cpu) {
-return;
+if (attrs.requester_type != MTRT_CPU) {
+return MEMTX_ACCESS_ERROR;
}
-
-cpu_synchronize_state(current_cpu);
-cpu = X86_CPU(current_cpu);
+cs = qemu_get_cpu(attrs.requester_id);
+cpu_synchronize_state(cs);
+cpu = X86_CPU(cs);
env = &cpu->env;
/*
@@ -708,6 +709,8 @@ static void vapic_write(void *opaque, hwaddr addr, uint64_t
data,
}
break;
}
+
+return MEMTX_OK;
}
static uint64_t vapic_read(void *opaque, hwaddr addr, unsigned size)
@@ -716,7 +719,7 @@ static uint64_t vapic_read(void *opaque, hwaddr addr,
unsigned size)
}
static const MemoryRegionOps vapic_ops = {
-.write = vapic_write,
+.write_with_attrs = vapic_write,
.read = vapic_read,
Shouldn't we do the same for the read() path?
.endianness = DEVICE_NATIVE_ENDIAN,
};
Re: [PATCH v5 19/20] hw/isa: derive CPUState from MemTxAttrs in apm_ioport_writeb
On 11/11/22 19:25, Alex Bennée wrote:
Some of the callbacks need a CPUState so extend the interface so we
can pass that down rather than relying on current_cpu hacks.
Signed-off-by: Alex Bennée
---
include/hw/isa/apm.h | 2 +-
hw/acpi/ich9.c | 1 -
hw/acpi/piix4.c | 2 +-
hw/isa/apm.c | 21 +
hw/isa/lpc_ich9.c| 5 ++---
5 files changed, 21 insertions(+), 10 deletions(-)
-static void apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
- unsigned size)
+static MemTxResult apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
+ unsigned size, MemTxAttrs attrs)
{
APMState *apm = opaque;
+CPUState *cs;
+
+if (attrs.requester_type != MTRT_CPU) {
+qemu_log_mask(LOG_UNIMP | LOG_GUEST_ERROR,
+ "%s: saw non-CPU transaction", __func__);
+return MEMTX_ACCESS_ERROR;
Are you sure it is illegal?
+}
+cs = qemu_get_cpu(attrs.requester_id);
+
addr &= 1;
trace_apm_io_write(addr, val);
@@ -41,11 +52,13 @@ static void apm_ioport_writeb(void *opaque, hwaddr addr,
uint64_t val,
apm->apmc = val;
if (apm->callback) {
-(apm->callback)(val, apm->arg);
+(apm->callback)(cs, val, apm->arg);
}
} else {
apm->apms = val;
}
+
+return MEMTX_OK;
}
Re: [PATCH v4] qapi/qom: Memory backend property prealloc-threads doc fix
On 14/11/22 04:24, Zhenyu Zhang wrote: Commit ffac16fab3 "hostmem: introduce "prealloc-threads" property" (v5.0.0) changed the default number of threads from number of CPUs to 1. This was deemed a regression, and fixed in commit f8d426a685 "hostmem: default the amount of prealloc-threads to smp-cpus". Except the documentation remained unchanged. Update 'qapi/qom.json' to reflect the change in commit f8d426a6852c. Signed-off-by: Zhenyu Zhang --- v4: Fix the line exceeding 80 characters limitation issue (Gavin) v3: Covers historical descriptions (Markus) v2: The property is changed to smp-cpus since 5.0 (Phild) --- qapi/qom.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] hw/intc: sifive_plic: Renumber the S irqs for numa support
On 14/11/22 11:34, Frédéric Pétrot wrote:
Le 14/11/2022 à 09:40, Philippe Mathieu-Daudé a écrit :
On 11/11/22 13:19, Frédéric Pétrot wrote:
Eventually we could unify the style:
-- >8 --
@@ -476,11 +476,11 @@ DeviceState *sifive_plic_create(hwaddr addr,
char *hart_config,
CPUState *cpu = qemu_get_cpu(cpu_num);
if (plic->addr_config[i].mode == PLICMode_M) {
- qdev_connect_gpio_out(dev, num_harts - plic->hartid_base
+ cpu_num,
+ qdev_connect_gpio_out(dev, cpu_num - hartid_base +
num_harts,
qdev_get_gpio_in(DEVICE(cpu),
IRQ_M_EXT));
}
if (plic->addr_config[i].mode == PLICMode_S) {
- qdev_connect_gpio_out(dev, cpu_num,
+ qdev_connect_gpio_out(dev, cpu_num - hartid_base,hartid_base
qdev_get_gpio_in(DEVICE(cpu),
IRQ_S_EXT));
}
}
---
IIUC hartid_base is used to set plic->hartid_base, so agreed, along
with the
style unification.
I'll send a v2, then.
Since Alistair already queued the patch, how shall I proceed?
I didn't notice Alistair queued (he usually send a notification by
responding "queued" to the patches). If it is queued, then too late
(and not a big deal) -- you can still post the v2 and let him pick
it :)
Re: [PATCH v2 02/12] tests/avocado: improve behaviour waiting for login prompts
On 14/11/22 17:28, Peter Maydell wrote:
On Fri, 11 Nov 2022 at 14:58, Alex Bennée wrote:
This attempts to deal with the problem of login prompts not being
guaranteed to be terminated with a newline. The solution to this is to
peek at the incoming data looking to see if we see an up-coming match
before we fall back to the old readline() logic. The reason to mostly
rely on readline is because I am occasionally seeing the peek stalling
despite data being there.
This seems kinda hacky and gross so I'm open to alternative approaches
and cleaner python code.
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
With this patch, the evb_sdk test fails:
Fetching asset from
./build/arm-clang/tests/avocado/machine_aspeed.py:AST2x00MachineSDK.test_arm_ast2500_evb_sdk
JOB ID : 542e050c4f7e1ddd6d5cdd350e4c26e1bdfcdee4
JOB LOG:
/home/petmay01/avocado/job-results/job-2022-11-14T16.21-542e050/job.log
(1/1)
./build/arm-clang/tests/avocado/machine_aspeed.py:AST2x00MachineSDK.test_arm_ast2500_evb_sdk:
ERROR: log() missing 1 required positional argument: 'msg' (82.57 s)
RESULTS: PASS 0 | ERROR 1 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0
| CANCEL 0
JOB TIME : 84.09 s
The avocado log reports a traceback where Python has thrown a
UnicodeDecodeError, and then subsequently an attempted debug
message in the error-handling path has a syntax error
("log() missing 1 required positional argument"):
_console_interaction(test, success_message, failure_message, None,
vm=vm)
2022-11-14 16:22:48,573 stacktrace L0045 ERROR| File
"/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/tests/avocad
o/avocado_qemu/__init__.py", line 226, in _console_interaction
2022-11-14 16:22:48,573 stacktrace L0045 ERROR| msg =
_peek_ahead(console, min_match, success_message)
2022-11-14 16:22:48,573 stacktrace L0045 ERROR| File
"/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/tests/avocad
o/avocado_qemu/__init__.py", line 180, in _peek_ahead
2022-11-14 16:22:48,573 stacktrace L0045 ERROR|
console_logger.log("error in decode of peek")
2022-11-14 16:22:48,573 stacktrace L0045 ERROR| TypeError: log()
missing 1 required positional argument: 'msg'
Indeed, log() expects a Level as first argument. Here we simply want to
report the exception as a warning and continue:
-- >8 --
except UnicodeDecodeError:
-console_logger.log("error in decode of peek")
+console_logger.warning("error in decode of peek")
return None
---
Re: [PATCH] hw/intc: sifive_plic: Renumber the S irqs for numa support
On 11/11/22 13:19, Frédéric Pétrot wrote:
Commit 40244040 changed the way the S irqs are numbered. This breaks when
40244040a7 in case?
using numa configuration, e.g.:
./qemu-system-riscv64 -nographic -machine virt,dumpdtb=numa-tree.dtb \
-m 2G -smp cpus=16 \
-object memory-backend-ram,id=mem0,size=512M \
-object memory-backend-ram,id=mem1,size=512M \
-object memory-backend-ram,id=mem2,size=512M \
-object memory-backend-ram,id=mem3,size=512M \
-numa node,cpus=0-3,memdev=mem0,nodeid=0 \
-numa node,cpus=4-7,memdev=mem1,nodeid=1 \
-numa node,cpus=8-11,memdev=mem2,nodeid=2 \
-numa node,cpus=12-15,memdev=mem3,nodeid=3
leads to:
Unexpected error in object_property_find_err() at ../qom/object.c:1304:
qemu-system-riscv64: Property 'riscv.sifive.plic.unnamed-gpio-out[8]' not
found
This patch makes the nubering of the S irqs identical to what it was before.
Signed-off-by: Frédéric Pétrot
---
hw/intc/sifive_plic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
index c2dfacf028..89d2122742 100644
--- a/hw/intc/sifive_plic.c
+++ b/hw/intc/sifive_plic.c
@@ -480,7 +480,7 @@ DeviceState *sifive_plic_create(hwaddr addr, char
*hart_config,
qdev_get_gpio_in(DEVICE(cpu), IRQ_M_EXT));
}
if (plic->addr_config[i].mode == PLICMode_S) {
-qdev_connect_gpio_out(dev, cpu_num,
+qdev_connect_gpio_out(dev, cpu_num - plic->hartid_base,
qdev_get_gpio_in(DEVICE(cpu), IRQ_S_EXT));
}
}
Oops.
Eventually we could unify the style:
-- >8 --
@@ -476,11 +476,11 @@ DeviceState *sifive_plic_create(hwaddr addr, char
*hart_config,
CPUState *cpu = qemu_get_cpu(cpu_num);
if (plic->addr_config[i].mode == PLICMode_M) {
-qdev_connect_gpio_out(dev, num_harts - plic->hartid_base +
cpu_num,
+qdev_connect_gpio_out(dev, cpu_num - hartid_base + num_harts,
qdev_get_gpio_in(DEVICE(cpu),
IRQ_M_EXT));
}
if (plic->addr_config[i].mode == PLICMode_S) {
-qdev_connect_gpio_out(dev, cpu_num,
+qdev_connect_gpio_out(dev, cpu_num - hartid_base,
qdev_get_gpio_in(DEVICE(cpu),
IRQ_S_EXT));
}
}
---
Reviewed-by: Philippe Mathieu-Daudé
Re: aarch64 GitLab CI runner down
Hi Stefan, On 14/11/22 22:56, Stefan Hajnoczi wrote: Hi Alex and Richard, The aarch64 GitLab CI runner is down again. Are you able to restart it? Alex wrote to your RH account :/ This is a scheduled shutdown. Equinix is relocating the hardware and this runner will be down for (at least) 4 days. Any idea why it disconnects sometimes? Sometimes? Odd. Can we check on the GitLab concentrator logs when the runner is unreachable? Regards, Phil.
tests/avocado/machine_s390_ccw_virtio: Fedora test failing
Hi,
As of v7.2.0-rc0 I am getting:
(101/198)
tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora:
FAIL (23.51 s)
$ cat
test-results/101-tests_avocado_machine_s390_ccw_virtio.py_S390CCWVirtioMachine.test_s390x_fedora/debug.log
01:20:44 INFO | INIT 1-S390CCWVirtioMachine.test_s390x_fedora
01:20:44 DEBUG| PARAMS (key=timeout, path=*, default=120) => 120
01:20:44 DEBUG| Test metadata:
01:20:44 DEBUG| filename:
/builddir/qemu/tests/avocado/machine_s390_ccw_virtio.py
01:20:44 DEBUG| teststmpdir: /tmp/avocado_ccw348hc
01:20:44 DEBUG| workdir:
/tmp/tmp2qg0mcch/test-results/tmp_dirkh7k2yra/1-S390CCWVirtioMachine.test_s390x_fedora
01:20:44 INFO | START 1-S390CCWVirtioMachine.test_s390x_fedora
01:20:44 DEBUG| DATA (filename=output.expected) => NOT FOUND (data
sources: variant, test, file)
01:20:44 DEBUG| PARAMS (key=arch, path=*, default=s390x) => 's390x'
01:20:44 DEBUG| PARAMS (key=cpu, path=*, default=None) => None
01:20:44 DEBUG| PARAMS (key=qemu_bin, path=*,
default=./qemu-system-s390x) => './qemu-system-s390x'
01:20:44 DEBUG| PARAMS (key=machine, path=*, default=s390-ccw-virtio) =>
's390-ccw-virtio'
01:20:46 DEBUG| QEMUMachine "default" created
01:20:46 DEBUG| QEMUMachine "default" temp_dir:
/tmp/tmp2qg0mcch/test-results/tmp_dirkh7k2yra/1-S390CCWVirtioMachine.test_s390x_fedora/qemu-machine-da4ubla3
01:20:46 DEBUG| QEMUMachine "default" log_dir:
/tmp/tmp2qg0mcch/test-results/1-S390CCWVirtioMachine.test_s390x_fedora
01:21:05 INFO | Test whether QEMU CLI options have been considered
01:21:06 INFO | Test screendump of virtio-gpu device
01:21:07 ERROR|
01:21:07 ERROR| Reproduced traceback from:
/builddir/qemu/tests/venv/lib/python3.10/site-packages/avocado/core/test.py:772
01:21:07 ERROR| Traceback (most recent call last):
01:21:07 ERROR| File
"/builddir/qemu/tests/venv/lib/python3.10/site-packages/avocado/core/decorators.py",
line 90, in wrapper
01:21:07 ERROR| return function(obj, *args, **kwargs)
01:21:07 ERROR| File
"/builddir/qemu/tests/avocado/machine_s390_ccw_virtio.py", line 256, in
test_s390x_fedora
01:21:07 ERROR| self.assertEqual(line, b"The quick fox jumps over a
lazy dog\n")
01:21:07 ERROR| File
"/opt/homebrew/Cellar/python@3.10/3.10.8/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py",
line 845, in assertEqual
01:21:07 ERROR| assertion_func(first, second, msg=msg)
01:21:07 ERROR| File
"/opt/homebrew/Cellar/python@3.10/3.10.8/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py",
line 838, in _baseAssertEqual
01:21:07 ERROR| raise self.failureException(msg)
01:21:07 ERROR| AssertionError:
b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\[979 chars]\x00' != b'The
quick fox jumps over a lazy dog\n'
01:21:07 ERROR|
01:21:07 DEBUG| Local variables:
01:21:07 DEBUG| -> obj 'machine_s390_ccw_virtio.S390CCWVirtioMachine'>:
1-S390CCWVirtioMachine.test_s390x_fedora
01:21:07 DEBUG| -> args : ()
01:21:07 DEBUG| -> kwargs : {}
01:21:07 DEBUG| -> condition : None
01:21:07 DEBUG| -> function : S390CCWVirtioMachine.test_s390x_fedora at 0x1063b1990>
01:21:07 DEBUG| -> message : Running on GitLab
01:21:07 DEBUG| -> negate : False
01:21:07 ERROR| FAIL 1-S390CCWVirtioMachine.test_s390x_fedora ->
AssertionError: b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\[979
chars]\x00' != b'The quick fox jumps over a lazy dog\n'
Re: [PATCH for-7.2] Add G_GNUC_PRINTF to function qemu_set_info_str and fix related issues
On 15/11/22 08:19, Stefan Weil via wrote: With the G_GNUC_PRINTF function attribute the compiler detects two potential insecure format strings: ../../../net/stream.c:248:31: warning: format string is not a string literal (potentially insecure) [-Wformat-security] qemu_set_info_str(&s->nc, uri); ^~~ ../../../net/stream.c:322:31: warning: format string is not a string literal (potentially insecure) [-Wformat-security] qemu_set_info_str(&s->nc, uri); ^~~ There are also two other warnings: ../../../net/socket.c:182:35: warning: zero-length gnu_printf format string [-Wformat-zero-length] 182 | qemu_set_info_str(&s->nc, ""); | ^~ ../../../net/stream.c:170:35: warning: zero-length gnu_printf format string [-Wformat-zero-length] 170 | qemu_set_info_str(&s->nc, ""); Signed-off-by: Stefan Weil --- include/net/net.h | 3 ++- net/socket.c | 2 +- net/stream.c | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: tests/avocado/machine_s390_ccw_virtio: Fedora test failing
On 15/11/22 12:05, Thomas Huth wrote: On 15/11/2022 12.03, Philippe Mathieu-Daudé wrote: Hi, As of v7.2.0-rc0 I am getting: (101/198) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: FAIL (23.51 s) Is it 100% reproducible? ... the test is known to be a little bit shaky, that's also why it is disabled in the gitlab CI. I am running it on my workstation, not GitLab. (1/2) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices: PASS (7.69 s) (2/2) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: FAIL (23.07 s) (1/2) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices: PASS (6.63 s) (2/2) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: FAIL (24.27 s) (1/2) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices: PASS (5.39 s) (2/2) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: FAIL (24.23 s) (1/2) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_devices: PASS (6.65 s) (2/2) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: FAIL (23.55 s) 5/5 failures. I'll skip it locally (no need to send a patch) and we can have a look after the release. Regards, Phil.
Re: [PATCH v3 0/7] memory: prevent dma-reentracy issues
On 10/11/22 21:50, Stefan Hajnoczi wrote: Preventing this class of bugs is important but QEMU is currently frozen for the 7.2 release. I'm a little concerned about regressions in a patch series that changes core device emulation code. I'm waiting for Alex's MemTxRequesterType field addition in MemTxAttrs [1] lands to rework my previous approach using flatview_access_allowed() instead of access_with_adjusted_size() [2]. I haven't looked at this series in detail, but since the permission check is done on the Memory API layer, I might have missed something in my previous intent (by using the FlatView layer). [1] https://lore.kernel.org/qemu-devel/2022182535.64844-2-alex.ben...@linaro.org/ [2] https://lore.kernel.org/qemu-devel/20211215182421.418374-4-phi...@redhat.com/ I'll review the series on Monday and if anyone has strong opinions on whether to merge this into 7.2, please say so. My thoughts are that this should be merged in the 7.3 release cycle so there's time to work out any issues. Stefan
Re: [PATCH v4 4/7] target/arm: Enable TTBCR_EAE for ARMv8-R AArch32
On 14/11/22 18:19, Peter Maydell wrote:
On Sun, 23 Oct 2022 at 16:37, wrote:
From: Tobias Röhmel
ARMv8-R AArch32 CPUs behave as if TTBCR.EAE is always 1 even
tough they don't have the TTBCR register.
See ARM Architecture Reference Manual Supplement - ARMv8, for the ARMv8-R
AArch32 architecture profile Version:A.c section C1.2.
Signed-off-by: Tobias Röhmel
---
target/arm/debug_helper.c | 3 +++
target/arm/internals.h| 4
target/arm/tlb_helper.c | 3 +++
3 files changed, 10 insertions(+)
diff --git a/target/arm/debug_helper.c b/target/arm/debug_helper.c
index c21739242c..73665f988b 100644
--- a/target/arm/debug_helper.c
+++ b/target/arm/debug_helper.c
@@ -437,6 +437,9 @@ static uint32_t arm_debug_exception_fsr(CPUARMState *env)
if (target_el == 2 || arm_el_is_aa64(env, target_el)) {
using_lpae = true;
+} else if (arm_feature(env, ARM_FEATURE_PMSA)
+&& arm_feature(env, ARM_FEATURE_V8)) {
Indentation looks wrong here. Generally the second line of a
multiline if (...) condition starts in the column after the '(',
so it lines up with the first part of the condition.
+using_lpae = true;
} else {
if (arm_feature(env, ARM_FEATURE_LPAE) &&
(env->cp15.tcr_el[target_el] & TTBCR_EAE)) {
For instance this is an example in the existing code.
We are inconsistent about whether we put operators like '&&' at
the end of the first line or beginning of the second line, so
pick whichever you like best, I guess.
Personally I find the operator at the end aesthetically nicer, but
few years ago Eric Blake reasoned that moving it at the beginning
was more explicit (to reviewers) thus safer, and I now I tend to
place it at the beginning.
Maybe part of the justification was cases where copy/pasting new
conditions in pre-existing could introduce a bug when the operator
is at the end?
+Eric/Daniel who usually give such good style advises :)
Re: [PATCH] hw/loongarch: Add cfi01 pflash device
On 15/11/22 12:56, Xiaojuan Yang wrote:
Add cfi01 pflash device for LoongArch virt machine
So the subject prefix should be "hw/loongarch/virt:".
Signed-off-by: Xiaojuan Yang
---
hw/loongarch/Kconfig| 1 +
hw/loongarch/acpi-build.c | 39 +++
hw/loongarch/virt.c | 130 +---
include/hw/loongarch/virt.h | 7 ++
4 files changed, 168 insertions(+), 9 deletions(-)
static bool memhp_type_supported(DeviceState *dev)
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 45c383f5a7..4ec4a7b4fe 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -12,6 +12,7 @@
#include "hw/boards.h"
#include "qemu/queue.h"
#include "hw/intc/loongarch_ipi.h"
+#include "hw/block/flash.h"
#define LOONGARCH_MAX_VCPUS 4
@@ -20,6 +21,11 @@
#define VIRT_FWCFG_BASE 0x1e02UL
#define VIRT_BIOS_BASE 0x1c00UL
#define VIRT_BIOS_SIZE (4 * MiB)
+#define VIRT_FLASH_SECTOR_SIZE (128 * KiB)
+#define VIRT_FLASH0_BASEVIRT_BIOS_BASE
+#define VIRT_FLASH0_SIZE(4 * MiB)
+#define VIRT_FLASH1_BASE(VIRT_FLASH0_BASE + VIRT_FLASH0_SIZE)
+#define VIRT_FLASH1_SIZE(4 * MiB)
#define VIRT_LOWMEM_BASE0
#define VIRT_LOWMEM_SIZE0x1000
@@ -48,6 +54,7 @@ struct LoongArchMachineState {
int fdt_size;
DeviceState *platform_bus_dev;
PCIBus *pci_bus;
+PFlashCFI01 *flash[2];
};
Since you are starting a virtual machine from scratch, you should take
the opportunity to learn from other early mistakes. X86 ended that way
due to 1/ old firmwares back-compability and 2/ QEMU pflash block
protections not being implemented. IIUC if we were starting with a
UEFI firmware today, the layout design (still using QEMU) would be
to map the CODE area in a dumb ROM device, and the VARSTORE area
in a PFlash device. Since Virt machines don't need to use Capsule
update, having the CODE area in ROM drastically simplifies the design
and maintainance.
Regards,
Phil.
Re: [PULL v4 00/83] pci,pc,virtio: features, tests, fixes, cleanups
Hi, On 7/11/22 23:47, Michael S. Tsirkin wrote: pci,pc,virtio: features, tests, fixes, cleanups lots of acpi rework first version of biosbits infrastructure ASID support in vhost-vdpa core_count2 support in smbios PCIe DOE emulation virtio vq reset HMAT support part of infrastructure for viommu support in vhost-vdpa VTD PASID support fixes, tests all over the place Apparently unrelated to these fixes, but going from 6295a58ad1 to v7.2.0-rc0 triggered rebuilding ACPI files and I now get: 45/510 qemu:qtest+qtest-i386 / qtest-i386/bios-tables-test ERROR 14.73s killed by signal 6 SIGABRT 74/510 qemu:qtest+qtest-x86_64 / qtest-x86_64/bios-tables-test ERROR 12.56s killed by signal 6 SIGABRT Running manually: $ QTEST_QEMU_BINARY=./qemu-system-x86_64 tests/qtest/bios-tables-test ... # starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-239233.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-239233.qmp,id=char0 -mon chardev=char0,mode=control -display none -machine pc -accel kvm -accel tcg -net none -machine smm=off -drive id=hd0,if=none,file=tests/acpi-test-disk-QmvOOR,format=raw -device ide-hd,drive=hd0 -accel qtest Could not access KVM kernel module: Permission denied qemu-system-x86_64: -accel kvm: failed to initialize kvm: Permission denied acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-Y06RV1], Expected [aml:tests/data/acpi/pc/DSDT.nosmm]. See source file tests/qtest/bios-tables-test.c for instructions on how to update expected files. to see ASL diff between mismatched files install IASL, rebuild QEMU from scratch and re-run tests with V=1 environment variable set** ERROR:../../tests/qtest/bios-tables-test.c:533:test_acpi_asl: assertion failed: (all_tables_match) Bail out! ERROR:../../tests/qtest/bios-tables-test.c:533:test_acpi_asl: assertion failed: (all_tables_match) Aborted I blew/recreated my build directory and can reproduce. $ uname -sm Linux x86_64 Any clue?
Re: [PATCH 0/1] hw/arm/aspeed: Automatically zero-extend flash images
On 15/11/22 14:06, Cédric Le Goater wrote:
Hello Peter,
On 11/14/22 20:08, Peter Delevoryas wrote:
I've been using this patch for a long time so that I don't have to use
dd to zero-extend stuff all the time. It's just doing what people are
doing already, right? I hope it's not controversial.
I simply run :
truncate --size
on the FW file when needed and it is rare because most FW image builders
know the flash size of the target.
However, the current error message is confusing and the following could
be an improvement :
@@ -1606,6 +1606,13 @@ static void m25p80_realize(SSIPeripheral
if (s->blk) {
uint64_t perm = BLK_PERM_CONSISTENT_READ |
(blk_supports_write_perm(s->blk) ?
BLK_PERM_WRITE : 0);
+
+ if (blk_getlength(s->blk) != s->size) {
'!=' -> '<' ?
+ error_setg(errp, "backend file is too small for flash
device %s (%d MB)",
+ object_class_get_name(OBJECT_CLASS(mc)), s->size
>> 20);
+ return;
+ }
+
ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);
if (ret < 0) {
return;
I can send a patch for the above.
I mostly run the QEMU machines with -snapshot, this hack :
blk_set_allow_write_beyond_eof(s->blk, true);
makes it work also ...
Thanks,
C.
Re: [PATCH v1 1/2] hw/intc: clean-up access to GIC multi-byte registers
On 15/11/22 14:40, Alex Bennée wrote: gic_dist_readb was returning a word value which just happened to work as a result of the way we OR the data together. Lets fix it so only the explicit byte is returned for each part of GICD_TYPER. I've changed the return type to uint8_t although the overflow is only detected with an explicit -Wconversion. Signed-off-by: Alex Bennée Signed-off-by: Peter Maydell --- hw/intc/arm_gic.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2] m25p80: Improve error when the backend file size does not match the device
On 15/11/22 16:10, Cédric Le Goater wrote: Currently, when a block backend is attached to a m25p80 device and the associated file size does not match the flash model, QEMU complains with the error message "failed to read the initial flash content". This is confusing for the user. Use blk_check_size_and_read_all() instead of blk_pread() to improve the reported error. Signed-off-by: Cédric Le Goater --- hw/block/m25p80.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 2/2] hw/intc: add implementation of GICD_IIDR to Arm GIC
On 15/11/22 17:17, Alex Bennée wrote:
a66a24585f (hw/intc/arm_gic: Implement read of GICC_IIDR) implemented
this for the CPU interface register. The fact we don't implement it
shows up when running Xen with -d guest_error which is definitely
wrong because the guest is perfectly entitled to read it.
Signed-off-by: Alex Bennée
---
v2
- checkpatch fixes.
v3
- re-base on re-flow with if
v4
- fix the commit message
---
hw/intc/arm_gic.c | 12 +++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 1a04144c38..7a34bc0998 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -973,8 +973,18 @@ static uint8_t gic_dist_readb(void *opaque, hwaddr offset,
MemTxAttrs attrs)
/* GICD_TYPER byte 1 */
return (s->security_extn << 2);
}
-if (offset < 0x08)
+if (offset == 8) {
+/* GICD_IIDR byte 0 */
+return 0x3b; /* Arm JEP106 identity */
+}
+if (offset == 9) {
+/* GICD_IIDR byte 1 */
+return 0x04; /* Arm JEP106 identity */
Possible future cleanup, define JEP106_ID_ARM:
$ git grep 0x43b
hw/intc/arm_gic.c:1671:*data = (s->revision << 16) | 0x43b;
hw/intc/gicv3_internal.h:743:return 0x43b;
hw/misc/armv7m_ras.c:26:*data = 0x43b;
+}
+if (offset < 0x0c) {
+/* All other bytes in this range are RAZ */
return 0;
+ }
Reviewed-by: Philippe Mathieu-Daudé
Re: tests/avocado/machine_s390_ccw_virtio: Fedora test failing
Cc'ing Jan/Cleber/Beraldo. On 16/11/22 10:43, Thomas Huth wrote: On 15/11/2022 12.13, Philippe Mathieu-Daudé wrote: On 15/11/22 12:05, Thomas Huth wrote: On 15/11/2022 12.03, Philippe Mathieu-Daudé wrote: Hi, As of v7.2.0-rc0 I am getting: (101/198) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: FAIL (23.51 s) Is it 100% reproducible? ... the test is known to be a little bit shaky, that's also why it is disabled in the gitlab CI. I am running it on my workstation, not GitLab. I just double-checked and for me, it's working fine an my laptop, with both, rc0 and rc1. 5/5 failures. I'll skip it locally (no need to send a patch) and we can have a look after the release. If it is a real bug, we should fix it before the release. Could you maybe bisect it, please? Also, what do you get when dumping the console? I.e.: ./tests/venv/bin/avocado --show=console run -t arch:s390x \ tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora When running with the current (old) Avocado runner I get: Avocado crashed: TypeError: cannot pickle '_thread.RLock' object Please include the traceback info and command line used on your bug report Report bugs visiting https://github.com/avocado-framework/avocado/issues/new I can use the 'new' runner: $ TMPDIR=/tmp ./tests/venv/bin/avocado --show=app,console run --test-runner=nrunner -t arch:s390x tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora JOB ID : 2e62ee88c4f04926281f31ab696f4cd4703612f5 JOB LOG: /Users/philmd/avocado/job-results/job-2022-11-16T11.05-2e62ee8/job.log (1/1) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: STARTED (1/1) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: FAIL (24.63 s) RESULTS: PASS 0 | ERROR 0 | FAIL 1 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0 JOB TIME : 25.16 s but I'm almost blind: I don't get any console output. The only useful info I found is closer to stderr: $ cat /Users/philmd/avocado/job-results/job-2022-11-16T11.05-2e62ee8//test-results/1-tests_avocado_machine_s390_ccw_virtio.py_S390CCWVirtioMachine.test_s390x_fedora/debug.log 11:05:20 INFO | INIT 1-S390CCWVirtioMachine.test_s390x_fedora 11:05:20 DEBUG| PARAMS (key=timeout, path=*, default=120) => 120 11:05:20 DEBUG| Test metadata: 11:05:20 DEBUG| filename: /Users/philmd/source/qemu/build/tests/avocado/machine_s390_ccw_virtio.py 11:05:20 DEBUG| teststmpdir: /tmp/avocado_zswqnw_l 11:05:20 DEBUG| workdir: /tmp/tmppq8l13fn/test-results/tmp_dir2cjl2nd2/1-S390CCWVirtioMachine.test_s390x_fedora 11:05:20 INFO | START 1-S390CCWVirtioMachine.test_s390x_fedora 11:05:20 DEBUG| DATA (filename=output.expected) => NOT FOUND (data sources: variant, test, file) 11:05:20 DEBUG| PARAMS (key=arch, path=*, default=s390x) => 's390x' 11:05:20 DEBUG| PARAMS (key=cpu, path=*, default=None) => None 11:05:20 DEBUG| PARAMS (key=qemu_bin, path=*, default=./qemu-system-s390x) => './qemu-system-s390x' 11:05:20 DEBUG| PARAMS (key=machine, path=*, default=s390-ccw-virtio) => 's390-ccw-virtio' 11:05:22 DEBUG| QEMUMachine "default" created 11:05:22 DEBUG| QEMUMachine "default" temp_dir: /tmp/tmppq8l13fn/test-results/tmp_dir2cjl2nd2/1-S390CCWVirtioMachine.test_s390x_fedora/qemu-machine-di5d3r66 11:05:22 DEBUG| QEMUMachine "default" log_dir: /tmp/tmppq8l13fn/test-results/1-S390CCWVirtioMachine.test_s390x_fedora 11:05:40 INFO | Test whether QEMU CLI options have been considered 11:05:41 INFO | Test screendump of virtio-gpu device 11:05:44 ERROR| 11:05:44 ERROR| Reproduced traceback from: /Users/philmd/source/qemu/build/tests/venv/lib/python3.10/site-packages/avocado/core/test.py:772 11:05:44 ERROR| Traceback (most recent call last): 11:05:44 ERROR| File "/Users/philmd/source/qemu/build/tests/venv/lib/python3.10/site-packages/avocado/core/decorators.py", line 90, in wrapper 11:05:44 ERROR| return function(obj, *args, **kwargs) 11:05:44 ERROR| File "/Users/philmd/source/qemu/build/tests/avocado/machine_s390_ccw_virtio.py", line 256, in test_s390x_fedora 11:05:44 ERROR| self.assertEqual(line, b"The quick fox jumps over a lazy dog\n") 11:05:44 ERROR| File "/opt/homebrew/Cellar/python@3.10/3.10.8/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py", line 845, in assertEqual 11:05:44 ERROR| assertion_func(first, second, msg=msg) 11:05:44 ERROR| File "/opt/homebrew/Cellar/python@3.10/3.10.8/Frameworks/Python.framework/Versions/3.10/lib/python3.10/unittest/case.py", line 838, in _baseAssertEqual 11:05:44 ERROR| raise self.failureException(msg) 11:05:44 ERROR| AssertionError: b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\[979 chars]\x00' != b'The quick fox jum
Re: tests/avocado/machine_s390_ccw_virtio: Fedora test failing
On 16/11/22 11:58, Thomas Huth wrote: On 16/11/2022 11.23, Philippe Mathieu-Daudé wrote: Cc'ing Jan/Cleber/Beraldo. On 16/11/22 10:43, Thomas Huth wrote: On 15/11/2022 12.13, Philippe Mathieu-Daudé wrote: On 15/11/22 12:05, Thomas Huth wrote: On 15/11/2022 12.03, Philippe Mathieu-Daudé wrote: Hi, As of v7.2.0-rc0 I am getting: (101/198) tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora: FAIL (23.51 s) Is it 100% reproducible? ... the test is known to be a little bit shaky, that's also why it is disabled in the gitlab CI. I am running it on my workstation, not GitLab. I just double-checked and for me, it's working fine an my laptop, with both, rc0 and rc1. 5/5 failures. I'll skip it locally (no need to send a patch) and we can have a look after the release. If it is a real bug, we should fix it before the release. Could you maybe bisect it, please? Also, what do you get when dumping the console? I.e.: ./tests/venv/bin/avocado --show=console run -t arch:s390x \ tests/avocado/machine_s390_ccw_virtio.py:S390CCWVirtioMachine.test_s390x_fedora When running with the current (old) Avocado runner I get: Avocado crashed: TypeError: cannot pickle '_thread.RLock' object Please include the traceback info and command line used on your bug report Report bugs visiting https://github.com/avocado-framework/avocado/issues/new I can use the 'new' runner: $ TMPDIR=/tmp ./tests/venv/bin/avocado --show=app,console run --test-runner=nrunner -t arch:s390x Uh, don't do that. nrunner does not seem to be able to execute that test yet. Or did you also upgrade your avocado to a newer installation? ... in that case it sounds like you might have messed up your installation? Maybe try again with a fresh and clean checkout of the QEMU repository / a new build folder. Well the current (old) runner doesn't work on Darwin ARM64: Avocado crashed: TypeError: cannot pickle '_thread.RLock' object Using `TMPDIR=/tmp avocado --test-runner=nrunner` almost all other tests pass. I'll wait for Cleber's nrunner update before re-testing/bisecting.
Re: [PATCH-for-7.2] target/ppc: Fix build warnings when building with 'disable-tcg'
On 16/11/22 14:17, Vaibhav Jain wrote:
Kowshik reported that building qemu with GCC 12.2.1 for 'ppc64-softmmu'
target is failing due to following build warnings:
../target/ppc/cpu_init.c:7018:13: error: 'ppc_restore_state_to_opc' defined
but not used [-Werror=unused-function]
7018 | static void ppc_restore_state_to_opc(CPUState *cs,
Fix this by wrapping these function definitions in 'ifdef CONFIG_TCG' so that
they are only defined if qemu is compiled with '--enable-tcg'
Reported-by: Kowshik Jois B S
Signed-off-by: Vaibhav Jain
---
target/ppc/cpu_init.c| 2 ++
target/ppc/excp_helper.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 32e94153d1..cbf0081374 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -7015,6 +7015,7 @@ static vaddr ppc_cpu_get_pc(CPUState *cs)
return cpu->env.nip;
}
+#ifdef CONFIG_TCG
static void ppc_restore_state_to_opc(CPUState *cs,
const TranslationBlock *tb,
const uint64_t *data)
@@ -7023,6 +7024,7 @@ static void ppc_restore_state_to_opc(CPUState *cs,
cpu->env.nip = data[0];
}
+#endif /* CONFIG_TCG */
Oops sorry.
Fixes: 61bd1d2942 ("target/ppc: Convert to tcg_ops restore_state_to_opc")
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a05a2ed595..94adcb766b 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2842,6 +2842,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1,
target_ulong arg2,
#endif
#endif
+#ifdef CONFIG_TCG
static uint32_t helper_SIMON_LIKE_32_64(uint32_t x, uint64_t key, uint32_t
lane)
{
const uint16_t c = 0xfffc;
@@ -2924,6 +2925,7 @@ HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true)
HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false)
HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true)
HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false)
+#endif /* CONFIG_TCG */
Fixes: 670f1da374 ("target/ppc: Implement hashst and hashchk")
Hmm this is another fix... You could split your patch in 2,
but not a big deal. Regardless:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] target/ppc: Fix build warnings when building with 'disable-tcg'
On 16/11/22 16:20, Greg Kurz wrote: Hi Vaibhav, Nice to see some people are still building QEMU at IBM ;-) On Wed, 16 Nov 2022 18:47:43 +0530 Vaibhav Jain wrote: Kowshik reported that building qemu with GCC 12.2.1 for 'ppc64-softmmu' target is failing due to following build warnings: ../target/ppc/cpu_init.c:7018:13: error: 'ppc_restore_state_to_opc' defined but not used [-Werror=unused-function] 7018 | static void ppc_restore_state_to_opc(CPUState *cs, Fix this by wrapping these function definitions in 'ifdef CONFIG_TCG' so that they are only defined if qemu is compiled with '--enable-tcg' Interestingly this config isn't covered in .gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml.
Re: [PATCH 2/2] pci: drop redundant PCIDeviceClass::is_bridge field
On 16/11/22 16:27, Igor Mammedov wrote:
and use cast to TYPE_PCI_BRIDGE instead.
Signed-off-by: Igor Mammedov
---
include/hw/pci/pci.h | 11 +--
include/hw/pci/pci_bridge.h| 1 +
hw/acpi/pcihp.c| 3 +--
hw/i386/acpi-build.c | 5 ++---
hw/pci-bridge/cxl_downstream.c | 1 -
hw/pci-bridge/cxl_upstream.c | 1 -
hw/pci-bridge/i82801b11.c | 1 -
hw/pci-bridge/pci_bridge_dev.c | 1 -
hw/pci-bridge/pcie_pci_bridge.c| 1 -
hw/pci-bridge/pcie_root_port.c | 1 -
hw/pci-bridge/simba.c | 1 -
hw/pci-bridge/xio3130_downstream.c | 1 -
hw/pci-bridge/xio3130_upstream.c | 1 -
hw/pci-host/designware.c | 1 -
hw/pci-host/xilinx-pcie.c | 1 -
hw/pci/pci.c | 20 +---
hw/ppc/spapr_pci.c | 15 +--
17 files changed, 19 insertions(+), 47 deletions(-)
@@ -1090,9 +1088,10 @@ static PCIDevice *do_pci_register_device(PCIDevice
*pci_dev,
Error *local_err = NULL;
DeviceState *dev = DEVICE(pci_dev);
PCIBus *bus = pci_get_bus(pci_dev);
+bool is_bridge = IS_PCI_BRIDGE(pci_dev);
/* Only pci bridges can be attached to extra PCI root buses */
-if (pci_bus_is_root(bus) && bus->parent_dev && !pc->is_bridge) {
+if (pci_bus_is_root(bus) && bus->parent_dev && !IS_PCI_BRIDGE(pci_dev)) {
Can we use the recently assigned 'is_bridge' variable?
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé
Re: [RFC PATCH] tests/avocado: move aarch64 boot_linux tcg tests and slim down
On 17/11/22 13:19, Alex Bennée wrote: The boot_linux tests download and run a full cloud image boot and start a full distro. While the ability to test the full boot chain is worthwhile it is perhaps a little too heavy weight and causes issues in CI. Fix this by dropping the TCG tests in boot_linux and replacing them with a alpine linux ISO boot in machine_aarch64_virt. This boots a fully loaded -cpu max with all the bells and whistles in 41s on my machine. A full debug build takes around 250s on my machine so we set a more generous timeout to cover that. We do drop testing for lesser GIC versions although there is some coverage for that already in the boot_xen.py tests. If we want to introduce more comprehensive testing we can do it with a custom kernel and initrd rather than a full distro boot. Signed-off-by: Alex Bennée --- tests/avocado/boot_linux.py | 43 + tests/avocado/machine_aarch64_virt.py | 46 ++- 2 files changed, 53 insertions(+), 36 deletions(-) I'd rather keep the current tests but: 1/ rename them as '_cloud_image', 2/ skip them on CI and 3/ add the '_alpine' tests to run on CI.
Re: [PATCH] ci: replace x86_64 macos-11 with aarch64 macos-12
On 16/11/22 18:50, Daniel P. Berrangé wrote:
The Cirrus CI service has announced the intent to discontinue
support for x86_64 macOS CI runners. They already have aarch64
runners available and require all projects to switch to these
images before Jan 1st 2023. The different architecture is
merely determined by the image name requested.
For aarch64 they only support macOS 12 onwards. At the same
time our support policy only guarantees the most recent 2
major versions, so macOS 12 is already technically our min
version.
https://cirrus-ci.org/blog/2022/11/08/sunsetting-intel-macos-instances/
Signed-off-by: Daniel P. Berrangé
---
.gitlab-ci.d/cirrus.yml | 12 ++--
.gitlab-ci.d/cirrus/{macos-11.vars => macos-12.vars} | 12 ++--
tests/lcitool/libvirt-ci | 2 +-
tests/lcitool/refresh| 2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
rename .gitlab-ci.d/cirrus/{macos-11.vars => macos-12.vars} (74%)
Thanks!
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 3/3] hw/{misc, riscv}: pfsoc: add system controller as unimplemented
On 17/11/22 18:00, Conor Dooley wrote:
On Sat, Nov 12, 2022 at 01:34:15PM +, Conor Dooley wrote:
From: Conor Dooley
The system controller on PolarFire SoC is access via a mailbox. The
control registers for this mailbox lie in the "IOSCB" region & the
interrupt is cleared via write to the "SYSREG" region. It also has a
QSPI controller, usually connected to a flash chip, that is used for
storing FPGA bitstreams and used for In-Application Programming (IAP).
Linux has an implementation of the system controller, through which the
hwrng is accessed, leading to load/store access faults.
Add the QSPI as unimplemented and a very basic (effectively
unimplemented) version of the system controller's mailbox. Rather than
purely marking the regions as unimplemented, service the mailbox
requests by reporting failures and raising the interrupt so a guest can
better handle the lack of support.
Signed-off-by: Conor Dooley
---
hw/misc/mchp_pfsoc_ioscb.c | 59 -
hw/misc/mchp_pfsoc_sysreg.c | 19 --
hw/riscv/microchip_pfsoc.c | 6 +++
include/hw/misc/mchp_pfsoc_ioscb.h | 3 ++
include/hw/misc/mchp_pfsoc_sysreg.h | 1 +
include/hw/riscv/microchip_pfsoc.h | 1 +
6 files changed, 83 insertions(+), 6 deletions(-)
@@ -143,6 +149,45 @@ static const MemoryRegionOps mchp_pfsoc_io_calib_ddr_ops =
{
.endianness = DEVICE_LITTLE_ENDIAN,
};
+#define SERVICES_SR 0x54
+
+static uint64_t mchp_pfsoc_ctrl_read(void *opaque, hwaddr offset,
+ unsigned size)
+{
+MchpPfSoCIoscbState *s = opaque;
+uint32_t val = 0;
+
+switch (offset) {
+case SERVICES_SR:
+/*
+ * Although some services have no error codes, most do. All services
+ * that do implement errors, begin their error codes at 1. Treat all
+ * service requests as failures & return 1.
+ * See the "PolarFire® FPGA and PolarFire SoC FPGA System Services"
+ * user guide for more information on service error codes.
+ */
+val = 1;
Another issue I just spotted here, this should not be returning 1, but
rather 1 << 16. The status bits are 31:16 & I was just getting lucky
b/c of something wrong with the Linux driver exercising it!
So your implementation expects 32-bit accesses, thus ...
+qemu_irq_raise(s->irq);
+break;
+default:
+qemu_log_mask(LOG_UNIMP, "%s: unimplemented device read "
+ "(size %d, offset 0x%" HWADDR_PRIx ")\n",
+ __func__, size, offset);
+}
+
+return val;
+}
+
+/*
+ * use the dummy write, since we are always going to report a failed message
+ * and therefore do not care what service is actually requested
+ */
+static const MemoryRegionOps mchp_pfsoc_ctrl_ops = {
+.read = mchp_pfsoc_ctrl_read,
+.write = mchp_pfsoc_dummy_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
... you might want to complete this with (at least):
.impl.min_access_size = 4,
And eventually if this region is only accessible as 32-bit:
.valid.min_access_size = 4,
+};
Re: [PATCH v1 1/2] virtio-gpu: Provide position info (x, y) to the Guest
On 18/11/22 02:37, Vivek Kasireddy wrote: While filling out the display info such as width, height to be provided to the Guest, make sure that the position information (x, y) is also included. This position info corresponds with the x and y fields mentioned in the spec: https://github.com/oasis-tcs/virtio-spec/blob/master/virtio-gpu.tex#L343 Cc: Dongwon Kim Cc: Gerd Hoffmann Signed-off-by: Vivek Kasireddy --- hw/display/virtio-gpu-base.c | 2 ++ 1 file changed, 2 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-7.2 v3 1/3] rtl8139: avoid clobbering tx descriptor bits
On 17/11/22 17:55, Stefan Hajnoczi wrote: The device turns the Tx Descriptor into a Tx Status descriptor after fully reading the descriptor. This involves clearing Tx Own (bit 31) to indicate that the driver has ownership of the descriptor again as well as several other bits. The code keeps the first dword of the Tx Descriptor in the txdw0 local variable. txdw0 is reused to build the first word of the Tx Status descriptor. Later on the code uses txdw0 again, incorrectly assuming that it still contains the first dword of the Tx Descriptor. The tx offloading code misbehaves because it sees bogus bits in txdw0. Use a separate local variable for Tx Status and preserve Tx Descriptor in txdw0. Signed-off-by: Stefan Hajnoczi --- hw/net/rtl8139.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-7.2 v3 3/3] rtl8139: honor large send MSS value
On 17/11/22 17:55, Stefan Hajnoczi wrote: The Large-Send Task Offload Tx Descriptor (9.2.1 Transmit) has a Large-Send MSS value where the driver specifies the MSS. See the datasheet here: http://realtek.info/pdf/rtl8139cp.pdf The code ignores this value and uses a hardcoded MSS of 1500 bytes instead. When the MTU is less than 1500 bytes the hardcoded value results in IP fragmentation and poor performance. Use the Large-Send MSS value to correctly size Large-Send packets. Jason Wang noticed that the Large-Send MSS value mask was incorrect so it is adjusted to match the datasheet and Linux 8139cp driver. This issue was discussed in the past here: https://lore.kernel.org/all/20161114162505.GD26664@stefanha-x1.localdomain/ Reported-by: Russell King - ARM Linux Reported-by: Tobias Fiebig Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1312 Signed-off-by: Stefan Hajnoczi --- hw/net/rtl8139.c | 26 -- 1 file changed, 12 insertions(+), 14 deletions(-) /* IP checksum offload flag */ #define CP_TX_IPCS (1<<18) @@ -2152,10 +2152,11 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) goto skip_offload; } -int large_send_mss = (txdw0 >> 16) & CP_TC_LGSEN_MSS_MASK; +int large_send_mss = (txdw0 >> CP_TC_LGSEN_MSS_SHIFT) & + CP_TC_LGSEN_MSS_MASK; Nitpicking/matter of style, the '&' is harder to miss if moved on the next line just before the mask. Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-7.2 v3 2/3] rtl8139: keep Tx command mode 0 and 1 separate
On 17/11/22 17:55, Stefan Hajnoczi wrote: There are two Tx Descriptor formats called mode 0 and mode 1. The mode is determined by the Large Send bit. CP_TX_IPCS (bit 18) is defined in mode 1 but the code checks the bit unconditionally. In mode 0 bit 18 is part of the Large Send MSS value. Explicitly check the Large Send bit to distinguish Tx command modes. This avoids bugs where modes are confused. Note that I didn't find any actual bugs aside from needlessly computing the IP checksum when the Large Send bit is enabled. Signed-off-by: Stefan Hajnoczi --- hw/net/rtl8139.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-7.2 v3 0/3] rtl8139: honor large send MSS value
On 17/11/22 17:55, Stefan Hajnoczi wrote: v3: - Add Patch 1 to avoid clobbering tx descriptor bits - Add Patch 2 to avoid confusing tx command modes - Exclude IP and TCP headers from large send MSS value Stefan Hajnoczi (3): rtl8139: avoid clobbering tx descriptor bits rtl8139: keep Tx command mode 0 and 1 separate rtl8139: honor large send MSS value hw/net/rtl8139.c | 47 --- 1 file changed, 24 insertions(+), 23 deletions(-) Per https://lore.kernel.org/qemu-devel/014101d8fac5$2cad8420$86088c60$@fiebig.nl/: Tested-by: Tobias Fiebig
Re: [PATCH v3 1/3] accel: introduce accelerator blocker API
On 11/11/22 16:47, Emanuele Giuseppe Esposito wrote:
This API allows the accelerators to prevent vcpus from issuing
new ioctls while execting a critical section marked with the
accel_ioctl_inhibit_begin/end functions.
Note that all functions submitting ioctls must mark where the
ioctl is being called with accel_{cpu_}ioctl_begin/end().
This API requires the caller to always hold the BQL.
API documentation is in sysemu/accel-blocker.h
Internally, it uses a QemuLockCnt together with a per-CPU QemuLockCnt
(to minimize cache line bouncing) to keep avoid that new ioctls
run when the critical section starts, and a QemuEvent to wait
that all running ioctls finish.
Signed-off-by: Emanuele Giuseppe Esposito
---
accel/accel-blocker.c | 154 +
accel/meson.build | 2 +-
hw/core/cpu-common.c | 2 +
include/hw/core/cpu.h | 3 +
include/sysemu/accel-blocker.h | 56
5 files changed, 216 insertions(+), 1 deletion(-)
create mode 100644 accel/accel-blocker.c
create mode 100644 include/sysemu/accel-blocker.h
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 1/3] accel: introduce accelerator blocker API
On 18/11/22 08:32, Philippe Mathieu-Daudé wrote:
On 11/11/22 16:47, Emanuele Giuseppe Esposito wrote:
This API allows the accelerators to prevent vcpus from issuing
new ioctls while execting a critical section marked with the
Typo "executing".
accel_ioctl_inhibit_begin/end functions.
Note that all functions submitting ioctls must mark where the
ioctl is being called with accel_{cpu_}ioctl_begin/end().
This API requires the caller to always hold the BQL.
API documentation is in sysemu/accel-blocker.h
Internally, it uses a QemuLockCnt together with a per-CPU QemuLockCnt
(to minimize cache line bouncing) to keep avoid that new ioctls
run when the critical section starts, and a QemuEvent to wait
that all running ioctls finish.
Signed-off-by: Emanuele Giuseppe Esposito
---
accel/accel-blocker.c | 154 +
accel/meson.build | 2 +-
hw/core/cpu-common.c | 2 +
include/hw/core/cpu.h | 3 +
include/sysemu/accel-blocker.h | 56
5 files changed, 216 insertions(+), 1 deletion(-)
create mode 100644 accel/accel-blocker.c
create mode 100644 include/sysemu/accel-blocker.h
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 04/13] docs/devel: add a maintainers section to development process
On 17/11/22 18:25, Alex Bennée wrote: We don't currently have a clear place in the documentation to describe the roles and responsibilities of a maintainer. Lets create one so we can. I've moved a few small bits out of other files to try and keep everything in one place. Signed-off-by: Alex Bennée Reviewed-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Message-Id: <2022145529.4020801-6-alex.ben...@linaro.org> --- docs/devel/code-of-conduct.rst | 2 + docs/devel/index-process.rst | 1 + docs/devel/maintainers.rst | 106 +++ docs/devel/submitting-a-pull-request.rst | 12 +-- 4 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 docs/devel/maintainers.rst +The Role of Maintainers +=== + +Maintainers are a critical part of the project's contributor ecosystem. +They come from a wide range of backgrounds from unpaid hobbyists +working in their spare time to employees who work on the project as +part of their job. Maintainer activities include: + + - reviewing patches and suggesting changes + - collecting patches and preparing pull requests + - tending to the long term health of their area + - participating in other project activities + +They are also human and subject to the same pressures as everyone else +including overload and burnout. Like everyone else they are subject +to project's :ref:`code_of_conduct` and should also be exemplars of +excellent community collaborators. + +The MAINTAINERS file + + +The `MAINTAINERS +<https://gitlab.com/qemu-project/qemu/-/blob/master/MAINTAINERS>`__ +file contains the canonical list of who is a maintainer. The file +is machine readable so an appropriately configured git (see +:ref:`cc_the_relevant_maintainer`) can automatically Cc them on +patches that touch their area of code. + +The file also describes the status of the area of code to give an idea +of how actively that section is maintained. + +.. list-table:: Meaning of support status in MAINTAINERS + :widths: 25 75 + :header-rows: 1 + + * - Status + - Meaning + * - Supported + - Someone is actually paid to look after this. + * - Maintained + - Someone actually looks after it. + * - Odd Fixes + - It has a maintainer but they don't have time to do + much other than throw the odd patch in. + * - Orphan + - No current maintainer. + * - Obsolete + - Old obsolete code, should use something else. We could add a line in MAINTAINERS 'Descriptions of section entries' header: "If you modify this section, please keep in sync with the description in docs/devel/maintainers.rst". +Becoming a reviewer +--- + +Most maintainers start by becoming subsystem reviewers. While anyone +is welcome to review code on the mailing list getting added to the +MAINTAINERS file with a line like:: + + R: Random Hacker + +will ensure that patches touching a given subsystem will automatically +be CC'd to you. Although 'R:' tag means 'designated reviewer', such reviewers is expected to provide regular spontaneous feedback. Otherwise being subscribed to the list is sufficient. Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 05/13] docs/devel: make language a little less code centric
On 17/11/22 18:25, Alex Bennée wrote: We welcome all sorts of patches. Signed-off-by: Alex Bennée Reviewed-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Message-Id: <2022145529.4020801-7-alex.ben...@linaro.org> --- docs/devel/submitting-a-patch.rst | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/devel/submitting-a-patch.rst b/docs/devel/submitting-a-patch.rst index fec33ce148..9c7c4331f3 100644 --- a/docs/devel/submitting-a-patch.rst +++ b/docs/devel/submitting-a-patch.rst @@ -3,11 +3,11 @@ Submitting a Patch == -QEMU welcomes contributions of code (either fixing bugs or adding new -functionality). However, we get a lot of patches, and so we have some -guidelines about submitting patches. If you follow these, you'll help -make our task of code review easier and your patch is likely to be -committed faster. +QEMU welcomes contributions to fix bugs, add functionality or improve +the documentation. However, we get a lot of patches, and so we have +some guidelines about submitting them. If you follow these, you'll +help make our task of code review easier and your patch is likely to +be committed faster. Less code centric: "... you'll help make our task of contribution review easier and your change is likely to be accepted and committed faster." Anyhow, Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 06/13] docs/devel: simplify the minimal checklist
On 17/11/22 18:25, Alex Bennée wrote: The bullet points are quite long and contain process tips. Move those bits of the bullet to the relevant sections and link to them. Use a table for nicer formatting of the checklist. Signed-off-by: Alex Bennée Reviewed-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Message-Id: <2022145529.4020801-8-alex.ben...@linaro.org> --- docs/devel/submitting-a-patch.rst | 75 --- 1 file changed, 49 insertions(+), 26 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH RFC 3/3] hw/nvme: add nvme management interface model
On 16/11/22 09:43, Klaus Jensen wrote: From: Klaus Jensen Add the 'nmi-i2c' device that emulates an NVMe Management Interface controller. Initial support is very basic (Read NMI DS, Configuration Get). This is based on previously posted code by Padmakar Kalghatgi, Arun Kumar Agasar and Saurav Kumar. Signed-off-by: Klaus Jensen --- hw/nvme/meson.build | 1 + hw/nvme/nmi-i2c.c| 381 +++ hw/nvme/trace-events | 6 + 3 files changed, 388 insertions(+) create mode 100644 hw/nvme/nmi-i2c.c +++ b/hw/nvme/nmi-i2c.c @@ -0,0 +1,381 @@ +/* + * SPDX-License-Identifier: GPL-2.0-only Just curious, is this restricted license choice on purpose? + * + * SPDX-FileCopyrightText: Copyright (c) 2022 Samsung Electronics Co., Ltd. + * + * SPDX-FileContributor: Padmakar Kalghatgi + * SPDX-FileContributor: Arun Kumar Agasar + * SPDX-FileContributor: Saurav Kumar + * SPDX-FileContributor: Klaus Jensen + */
Re: [PATCH v3 07/13] docs/devel: try and improve the language around patch review
On 17/11/22 18:25, Alex Bennée wrote: It is important that contributors take the review process seriously and we collaborate in a respectful way while avoiding personal attacks. Try and make this clear in the language. Signed-off-by: Alex Bennée Reviewed-by: Markus Armbruster Reviewed-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Message-Id: <2022145529.4020801-9-alex.ben...@linaro.org> --- docs/devel/submitting-a-patch.rst | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 09/13] tests/avocado: introduce alpine virt test for CI
On 17/11/22 18:25, Alex Bennée wrote:
The boot_linux tests download and run a full cloud image boot and
start a full distro. While the ability to test the full boot chain is
worthwhile it is perhaps a little too heavy weight and causes issues
in CI. Fix this by introducing a new alpine linux ISO boot in
machine_aarch64_virt.
This boots a fully loaded -cpu max with all the bells and whistles in
31s on my machine. A full debug build takes around 180s on my machine
so we set a more generous timeout to cover that.
We don't add a test for lesser GIC versions although there is some
coverage for that already in the boot_xen.py tests. If we want to
introduce more comprehensive testing we can do it with a custom kernel
and initrd rather than a full distro boot.
Signed-off-by: Alex Bennée
---
v1
- use "virt" image instead (even faster)
- don't drop boot_linux (it is now disabled for CI)
- re-phrase commit message
- add alpine to the test name
---
tests/avocado/machine_aarch64_virt.py | 46 ++-
1 file changed, 45 insertions(+), 1 deletion(-)
+# This tests the whole boot chain from EFI to Userspace
+# We only boot a whole OS for the current top level CPU and GIC
+# Other test profiles should use more minimal boots
+def test_alpine_virt_tcg_gic_max(self):
+"""
+:avocado: tags=arch:aarch64
+:avocado: tags=machine:virt
+:avocado: tags=accel:tcg
+"""
+iso_url = ('https://dl-cdn.alpinelinux.org/'
+ 'alpine/v3.16/releases/aarch64/'
+ 'alpine-virt-3.16.3-aarch64.iso')
+
+# Alpine use sha256 so I recalculated this myself
+iso_sha1 = '0683bc089486d55c91bf6607d5ecb93925769bc0'
+iso_path = self.fetch_asset(iso_url, asset_hash=iso_sha1)
+
+self.vm.set_console()
+kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
+ 'console=ttyAMA0')
+self.require_accelerator("tcg")
+
+self.vm.add_args("-accel", "tcg")
+self.vm.add_args("-cpu", "max,pauth-impdef=on")
+self.vm.add_args("-machine",
+ "virt,acpi=on,"
+ "virtualization=on,"
+ "mte=on,"
+ "gic-version=max,iommu=smmuv3")
+self.vm.add_args("-smp", "2", "-m", "1024")
+self.vm.add_args('-bios', os.path.join(BUILD_DIR, 'pc-bios',
+ 'edk2-aarch64-code.fd'))
I am not sure about restricting to BUILD_DIR, I'd rather use an
externally prebuilt image, i.e.:
https://snapshots.linaro.org/components/kernel/leg-virt-tianocore-edk2-upstream/4710/QEMU-AARCH64/RELEASE_GCC5/
(I'd like to use these Avocado tests with a distrib provided QEMU
binary).
Anyhow can be fixed later, so:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 10/13] tests/avocado: skip aarch64 cloud TCG tests in CI
On 17/11/22 18:25, Alex Bennée wrote: We now have a much lighter weight test in machine_aarch64_virt which tests the full boot chain in less time. Rename the tests while we are at it to make it clear it is a Fedora cloud image. Signed-off-by: Alex Bennée --- tests/avocado/boot_linux.py | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) Thanks for the update. Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v3 12/13] tests/avocado/boot_linux.py: Bump aarch64 virt test timeout to 720s
On 17/11/22 18:25, Alex Bennée wrote: From: Peter Maydell The two tests tests/avocado/boot_linux.py:BootLinuxAarch64.test_virt_tcg_gicv2 tests/avocado/boot_linux.py:BootLinuxAarch64.test_virt_tcg_gicv3 take quite a long time to run, and the current timeout of 240s is not enough for the tests to complete on slow machines: we've seen these tests time out in the gitlab CI in the 'avocado-system-alpine' CI job, for instance. The timeout The previous patches removed these jobs from GitLab CI, so this shouldn't be a problem there anymore, but the next part is still relevant: is also insufficient for running the test with a debug build of QEMU: on my machine the tests take over 10 minutes to run in that config. Reviewed-by: Philippe Mathieu-Daudé Push the timeout up to 720s so that the test definitely has enough time to complete. Signed-off-by: Peter Maydell Reviewed-by: Thomas Huth Message-Id: <20221117111628.911686-1-peter.mayd...@linaro.org> Signed-off-by: Alex Bennée --- tests/avocado/boot_linux.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Re: [PATCH for-8.0 3/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_maybe_interrupt
On 18/11/22 10:18, Richard Henderson wrote: Signed-off-by: Richard Henderson --- Cc: qemu-...@nongnu.org --- target/ppc/excp_helper.c | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 2/7] hw/mips: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_mips_irq_request
On 18/11/22 10:18, Richard Henderson wrote:
Signed-off-by: Richard Henderson
---
Cc: Philippe Mathieu-Daudé
Cc: Jiaxun Yang
---
hw/mips/mips_int.c | 11 +--
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/hw/mips/mips_int.c b/hw/mips/mips_int.c
index 2db5e10fe0..73437cd90f 100644
--- a/hw/mips/mips_int.c
+++ b/hw/mips/mips_int.c
@@ -32,17 +32,12 @@ static void cpu_mips_irq_request(void *opaque, int irq, int
level)
MIPSCPU *cpu = opaque;
CPUMIPSState *env = &cpu->env;
CPUState *cs = CPU(cpu);
-bool locked = false;
if (irq < 0 || irq > 7) {
return;
}
-/* Make sure locking works even if BQL is already held by the caller */
-if (!qemu_mutex_iothread_locked()) {
-locked = true;
-qemu_mutex_lock_iothread();
-}
+QEMU_IOTHREAD_LOCK_GUARD();
if (level) {
env->CP0_Cause |= 1 << (irq + CP0Ca_IP);
@@ -59,10 +54,6 @@ static void cpu_mips_irq_request(void *opaque, int irq, int
level)
} else {
cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
}
-
-if (locked) {
-qemu_mutex_unlock_iothread();
-}
}
I'd rather have a macro similar to WITH_RCU_READ_LOCK_GUARD()
so the locked context is obvious:
WITH_QEMU_IOTHREAD_LOCK_GUARD() {
...
}
Anyhow:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 5/7] target/riscv: Use QEMU_IOTHREAD_LOCK_GUARD in riscv_cpu_update_mip
On 18/11/22 10:18, Richard Henderson wrote: Signed-off-by: Richard Henderson --- Cc: Alistair Francis Cc: Bin Meng Cc: qemu-ri...@nongnu.org --- target/riscv/cpu_helper.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 4/7] target/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in cpu_interrupt_exittb
On 18/11/22 10:18, Richard Henderson wrote: In addition, use tcg_enabled instead of !kvm_enabled. Signed-off-by: Richard Henderson --- Cc: qemu-...@nongnu.org --- target/ppc/helper_regs.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 6/7] hw/ppc: Use QEMU_IOTHREAD_LOCK_GUARD in ppc_set_irq
On 18/11/22 10:18, Richard Henderson wrote: Signed-off-by: Richard Henderson --- Cc: qemu-...@nongnu.org --- hw/ppc/ppc.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 7/7] accel/tcg: Use QEMU_IOTHREAD_LOCK_GUARD in io_readx/io_writex
On 18/11/22 10:18, Richard Henderson wrote:
Narrow the scope of the lock to the actual read/write,
moving the cpu_transation_failed call outside the lock.
Signed-off-by: Richard Henderson
---
accel/tcg/cputlb.c | 25 -
1 file changed, 8 insertions(+), 17 deletions(-)
@@ -1367,11 +1366,11 @@ static uint64_t io_readx(CPUArchState *env,
CPUTLBEntryFull *full,
cpu_io_recompile(cpu, retaddr);
}
-if (!qemu_mutex_iothread_locked()) {
-qemu_mutex_lock_iothread();
-locked = true;
+{
+QEMU_IOTHREAD_LOCK_GUARD();
+r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs);
}
-r = memory_region_dispatch_read(mr, mr_offset, &val, op, full->attrs);
+
Example of clearer WITH_QEMU_IOTHREAD_LOCK_GUARD() macro
use suggested earlier:
WITH_QEMU_IOTHREAD_LOCK_GUARD() {
r = memory_region_dispatch_read(mr, mr_offset, &val,
op, full->attrs);
}
if (r != MEMTX_OK) {
hwaddr physaddr = mr_offset +
section->offset_within_address_space -
@@ -1380,10 +1379,6 @@ static uint64_t io_readx(CPUArchState *env,
CPUTLBEntryFull *full,
cpu_transaction_failed(cpu, physaddr, addr, memop_size(op),
access_type,
mmu_idx, full->attrs, r, retaddr);
}
-if (locked) {
-qemu_mutex_unlock_iothread();
-}
-
return val;
}
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD
On 21/11/22 10:55, Alex Bennée wrote: Richard Henderson writes: On 11/18/22 05:30, Alex Bennée wrote: Richard Henderson writes: Create a wrapper for locking/unlocking the iothread lock. Signed-off-by: Richard Henderson --- Cc: Paolo Bonzini (maintainer:Main loop) You might want to review Paolo's comments from: Subject: [RFC PATCH] main-loop: introduce WITH_QEMU_IOTHREAD_LOCK Date: Mon, 24 Oct 2022 18:19:09 +0100 Message-Id: <20221024171909.434818-1-alex.ben...@linaro.org> So it would be worth having the WITH_QEMU_IOTHREAD_LOCK() and MAYBE_WITH_QEMU_IOTHREAD_LOCK() helpers for completeness. I don't see that (MAYBE_)WITH_QEMU_IOTHREAD_LOCK is particularly useful in any of the cases that I converted. Fair enough - as long as they are easy enough to add later. The WITH_ forms do work nicely to wrap a particular area under lock and make things visually clear vs the LOCK_GUARD which basically holds the lock to the end of function or exit. I concur for WITH_QEMU_IOTHREAD_LOCK(), it is a no-brainer.
Re: [PATCH] tests/avocado: Update the URLs of the advent calendar images
On 21/11/22 11:24, Thomas Huth wrote:
The qemu-advent-calendar.org server will be decommissioned soon.
I've mirrored the images that we use for the QEMU CI to gitlab,
so update their URLs to point to the new location.
Signed-off-by: Thomas Huth
---
tests/avocado/boot_linux_console.py | 4 +--
tests/avocado/machine_arm_canona1100.py | 4 +--
tests/avocado/machine_microblaze.py | 4 +--
tests/avocado/machine_sparc64_sun4u.py | 4 +--
tests/avocado/ppc_mpc8544ds.py | 6 ++--
tests/avocado/ppc_virtex_ml507.py | 6 ++--
tests/avocado/replay_kernel.py | 40 -
7 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/tests/avocado/boot_linux_console.py
b/tests/avocado/boot_linux_console.py
index 4c9d551f47..f3e6f44ae9 100644
--- a/tests/avocado/boot_linux_console.py
+++ b/tests/avocado/boot_linux_console.py
@@ -1029,8 +1029,8 @@ def test_m68k_q800(self):
self.wait_for_console_pattern(console_pattern)
def do_test_advcal_2018(self, day, tar_hash, kernel_name, console=0):
-tar_url = ('https://www.qemu-advent-calendar.org'
- '/2018/download/day' + day + '.tar.xz')
+tar_url = ('https://qemu-advcal.gitlab.io'
+ '/qac-best-of-multiarch/download/day' + day + '.tar.xz')
You could insert the year in the url, so you can eventually add other
editions :)
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
Re: [PATCH 01/10] error: Drop some obviously superfluous error_propagate()
On 21/11/22 09:50, Markus Armbruster wrote: When error_propagate(errp, local_err) is the only reader of @local_err, we can just as well change its writers to write @errp directly, and drop the error_propagate() along with @local_err. Signed-off-by: Markus Armbruster --- hw/arm/virt.c| 14 +- hw/hyperv/vmbus.c| 8 +++- qga/commands-win32.c | 8 +++- 3 files changed, 11 insertions(+), 19 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 03/10] error: Move ERRP_GUARD() to the beginning of the function
On 21/11/22 09:50, Markus Armbruster wrote: include/qapi/error.h advises to put ERRP_GUARD() right at the beginning of the function, because only then can it guard the whole function. Clean up the few spots disregarding the advice. Signed-off-by: Markus Armbruster --- hw/arm/armsse.c| 3 +-- hw/core/machine.c | 3 +-- hw/virtio/vhost-vdpa.c | 2 +- iothread.c | 2 +- monitor/qmp-cmds.c | 4 ++-- 5 files changed, 6 insertions(+), 8 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 10/10] io: Tidy up fat-fingered parameter name
On 21/11/22 09:50, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- include/io/channel.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 09/10] qapi: Use returned bool to check for failure (again)
On 21/11/22 09:50, Markus Armbruster wrote: Commit 012d4c96e2 changed the visitor functions taking Error ** to return bool instead of void, and the commits following it used the new return value to simplify error checking. Since then a few more uses in need of the same treatment crept in. Do that. All pretty mechanical except for * balloon_stats_get_all() This is basically the same transformation commit 012d4c96e2 applied to the virtual walk example in include/qapi/visitor.h. * set_max_queue_size() Additionally replace "goto end of function" by return. Signed-off-by: Markus Armbruster --- accel/kvm/kvm-all.c | 5 + hw/core/qdev-properties-system.c | 5 + hw/i386/pc.c | 5 + hw/virtio/virtio-balloon.c | 20 +--- hw/virtio/virtio-mem.c | 10 ++ net/colo-compare.c | 13 - target/i386/kvm/kvm.c| 5 + util/thread-context.c| 10 ++ 8 files changed, 21 insertions(+), 52 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [RFC PATCH 1/1] Dirty quota-based throttling of vcpus
Hi,
On 20/11/22 23:54, Shivam Kumar wrote:
Introduces a (new) throttling scheme where QEMU defines a limit on the dirty
rate of each vcpu of the VM. This limit is enfored on the vcpus in small
intervals (dirty quota intervals) by allowing the vcpus to dirty only as many
pages in these intervals as to maintain a dirty rate below the set limit.
Suggested-by: Shaju Abraham
Suggested-by: Manish Mishra
Co-developed-by: Anurag Madnawat
Signed-off-by: Anurag Madnawat
Signed-off-by: Shivam Kumar
---
accel/kvm/kvm-all.c | 91 +++
include/exec/memory.h | 3 ++
include/hw/core/cpu.h | 5 +++
include/sysemu/kvm_int.h | 1 +
linux-headers/linux/kvm.h | 9
migration/migration.c | 22 ++
migration/migration.h | 31 +
softmmu/memory.c | 64 +++
8 files changed, 226 insertions(+)
void migrate_set_state(int *state, int old_state, int new_state);
diff --git a/softmmu/memory.c b/softmmu/memory.c
index bc0be3f62c..8f725a9b89 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -12,6 +12,7 @@
* Contributions after 2012-01-13 are licensed under the terms of the
* GNU GPL, version 2 or (at your option) any later version.
*/
+#include
#include "qemu/osdep.h"
#include "qemu/log.h"
@@ -34,6 +35,10 @@
#include "hw/boards.h"
#include "migration/vmstate.h"
#include "exec/address-spaces.h"
+#include "hw/core/cpu.h"
+#include "exec/target_page.h"
+#include "migration/migration.h"
+#include "sysemu/kvm_int.h"
//#define DEBUG_UNASSIGNED
@@ -2869,6 +2874,46 @@ static unsigned int postponed_stop_flags;
static VMChangeStateEntry *vmstate_change;
static void memory_global_dirty_log_stop_postponed_run(void);
+static void init_vcpu_dirty_quota(CPUState *cpu, run_on_cpu_data arg)
+{
+uint64_t current_time = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
+cpu->kvm_run->dirty_quota = 1;
+cpu->dirty_quota_expiry_time = current_time;
+}
+
+void dirty_quota_migration_start(void)
+{
+if (!kvm_state->dirty_quota_supported) {
You are accessing an accelerator-specific variable in an
accelerator-agnostic file, this doesn't sound correct.
You might introduce some hooks in AccelClass and implement them in
accel/kvm/. See for example gdbstub_supported_sstep_flags() and
kvm_gdbstub_sstep_flags().
+return;
+}
+
+MigrationState *s = migrate_get_current();
+/* Assuming initial bandwidth to be 128 MBps. */
+double pages_per_second = (((double) 1e9) / 8.0) /
+(double) qemu_target_page_size();
+uint32_t nr_cpus;
+CPUState *cpu;
+
+CPU_FOREACH(cpu) {
+nr_cpus++;
+}
+/*
+ * Currently we are hardcoding this to 2. There are plans to allow the user
+ * to manually select this ratio.
+ */
+s->dirty_quota_throttle_ratio = 2;
+qatomic_set(&s->per_vcpu_dirty_rate_limit,
+pages_per_second / s->dirty_quota_throttle_ratio / nr_cpus);
+
+qemu_spin_lock(&s->common_dirty_quota_lock);
+s->common_dirty_quota = 0;
+qemu_spin_unlock(&s->common_dirty_quota_lock);
+
+CPU_FOREACH(cpu) {
+run_on_cpu(cpu, init_vcpu_dirty_quota, RUN_ON_CPU_NULL);
+}
+}
+
void memory_global_dirty_log_start(unsigned int flags)
{
unsigned int old_flags;
@@ -2891,6 +2936,7 @@ void memory_global_dirty_log_start(unsigned int flags)
trace_global_dirty_changed(global_dirty_tracking);
if (!old_flags) {
+dirty_quota_migration_start();
MEMORY_LISTENER_CALL_GLOBAL(log_global_start, Forward);
memory_region_transaction_begin();
memory_region_update_pending = true;
@@ -2898,6 +2944,23 @@ void memory_global_dirty_log_start(unsigned int flags)
}
}
+static void reset_vcpu_dirty_quota(CPUState *cpu, run_on_cpu_data arg)
+{
+cpu->kvm_run->dirty_quota = 0;
+}
+
+void dirty_quota_migration_stop(void)
+{
+if (!kvm_state->dirty_quota_supported) {
+return;
+}
+
+CPUState *cpu;
+CPU_FOREACH(cpu) {
+run_on_cpu(cpu, reset_vcpu_dirty_quota, RUN_ON_CPU_NULL);
+}
+}
+
static void memory_global_dirty_log_do_stop(unsigned int flags)
{
assert(flags && !(flags & (~GLOBAL_DIRTY_MASK)));
@@ -2907,6 +2970,7 @@ static void memory_global_dirty_log_do_stop(unsigned int
flags)
trace_global_dirty_changed(global_dirty_tracking);
if (!global_dirty_tracking) {
+dirty_quota_migration_stop();
memory_region_transaction_begin();
memory_region_update_pending = true;
memory_region_transaction_commit();
Re: [PATCH for-8.0 01/29] include/qemu/cpuid: Introduce xgetbv_low
On 18/11/22 10:47, Richard Henderson wrote: Replace the two uses of asm to expand xgetbv with an inline function. Since one of the two has been using the mnemonic, assume that the comment about "older versions of the assember" is obsolete, as even that is 4 years old. Signed-off-by: Richard Henderson --- include/qemu/cpuid.h | 7 +++ util/bufferiszero.c | 3 +-- tcg/i386/tcg-target.c.inc | 11 --- 3 files changed, 12 insertions(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 04/29] accel/tcg: Introduce tlb_read_idx
On 18/11/22 10:47, Richard Henderson wrote: Instead of playing with offsetof in various places, use MMUAccessType to index an array. This is easily defined instead of the previous dummy padding array in the union. Signed-off-by: Richard Henderson --- include/exec/cpu-defs.h | 7 ++- include/exec/cpu_ldst.h | 26 -- accel/tcg/cputlb.c | 104 +--- 3 files changed, 59 insertions(+), 78 deletions(-) Nice. Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 09/29] tcg/tci: Use cpu_{ld,st}_mmu
On 18/11/22 10:47, Richard Henderson wrote: Unify the softmmu and the user-only paths by using the official memory interface. Avoid double logging of memory operations to plugins by relying on the ones within the cpu_*_mmu functions. Signed-off-by: Richard Henderson --- tcg/tcg-op.c | 9 +++- tcg/tci.c| 127 --- 2 files changed, 26 insertions(+), 110 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 10/29] tcg: Unify helper_{be,le}_{ld,st}*
On 18/11/22 10:47, Richard Henderson wrote: With the current structure of cputlb.c, there is no difference between the little-endian and big-endian entry points, aside from the assert. Unify the pairs of functions. Signed-off-by: Richard Henderson --- include/tcg/tcg-ldst.h | 60 -- accel/tcg/cputlb.c | 190 ++- docs/devel/loads-stores.rst | 36 ++ tcg/aarch64/tcg-target.c.inc | 39 +++ tcg/arm/tcg-target.c.inc | 45 +++- tcg/i386/tcg-target.c.inc| 40 +++ tcg/loongarch64/tcg-target.c.inc | 25 ++-- tcg/mips/tcg-target.c.inc| 40 +++ tcg/ppc/tcg-target.c.inc | 30 ++--- tcg/riscv/tcg-target.c.inc | 51 +++-- tcg/s390x/tcg-target.c.inc | 38 +++ tcg/sparc64/tcg-target.c.inc | 37 +++--- 12 files changed, 226 insertions(+), 405 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] cleanup: Tweak and re-run return_directly.cocci
On 21/11/22 15:36, Peter Maydell wrote: On Mon, 21 Nov 2022 at 14:03, Markus Armbruster wrote: Tweak the semantic patch to drop redundant parenthesis around the return expression. Coccinelle drops comments in hw/rdma/vmw/pvrdma_cmd.c; restored manually. Coccinelle messes up vmdk_co_create(), not sure why. Transformed manually. Line breaks in target/avr/cpu.h and hw/rdma/vmw/pvrdma_cmd.c tidied up manually. Whitespace in fuse_reply_iov() tidied up manually. checkpatch.pl complains "return of an errno should typically be -ve" two times for hw/9pfs/9p-synth.c. Preexisting, the patch merely makes it visible to checkpatch.pl. checkpatch.pl complains "return is not a function, parentheses are not required" three times for target/mips/tcg/dsp_helper.c. False positives. Signed-off-by: Markus Armbruster .../user/ase/msa/bit-count/test_msa_nloc_b.c | 9 +- .../user/ase/msa/bit-count/test_msa_nloc_d.c | 9 +- [snip long list of other mips test files] 328 files changed, 989 insertions(+), 2099 deletions(-) This patch seems to almost entirely be huge because of these mips test case files. Are they specific to QEMU or are they effectively a 3rd-party import that it doesn't make sense to make local changes to? They are imported and will unlikely be modified.
Re: [PATCH for-8.0 19/29] tcg: Introduce TCG_OPF_TYPE_MASK
On 18/11/22 10:47, Richard Henderson wrote: Reorg TCG_OPF_64BIT and TCG_OPF_VECTOR into a two-bit field so that we can add TCG_OPF_128BIT without requiring another bit. Signed-off-by: Richard Henderson --- include/tcg/tcg.h| 22 -- tcg/optimize.c | 15 --- tcg/tcg.c| 4 ++-- tcg/aarch64/tcg-target.c.inc | 8 +--- tcg/tci/tcg-target.c.inc | 3 ++- 5 files changed, 33 insertions(+), 19 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 21/29] tcg/i386: Introduce tcg_out_mov2
On 18/11/22 10:47, Richard Henderson wrote: Create a helper for data movement minding register overlap. Use the more general xchg instruction, which consumes one extra byte, but simplifies the more general function. Signed-off-by: Richard Henderson --- tcg/i386/tcg-target.c.inc | 27 +-- 1 file changed, 21 insertions(+), 6 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 22/29] tcg/i386: Introduce tcg_out_testi
On 18/11/22 10:47, Richard Henderson wrote: Split out a helper for choosing testb vs testl. Signed-off-by: Richard Henderson --- tcg/i386/tcg-target.c.inc | 30 ++ 1 file changed, 18 insertions(+), 12 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 24/29] tcg/i386: Replace is64 with type in qemu_ld/st routines
On 18/11/22 10:47, Richard Henderson wrote:
Prepare for TCG_TYPE_I128 by not using a boolean.
Signed-off-by: Richard Henderson
---
tcg/i386/tcg-target.c.inc | 54 ++-
1 file changed, 36 insertions(+), 18 deletions(-)
@@ -2315,7 +2324,7 @@ static void tcg_out_qemu_st_direct(TCGContext *s, TCGReg
datalo, TCGReg datahi,
}
}
-static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, bool is64)
+static void tcg_out_qemu_st(TCGContext *s, const TCGArg *args, TCGType type)
{
TCGReg datalo, datahi, addrlo;
TCGReg addrhi __attribute__((unused));
@@ -2327,7 +2336,16 @@ static void tcg_out_qemu_st(TCGContext *s, const TCGArg
*args, bool is64)
Confusing git-diff context :)
#endif
datalo = *args++;
-datahi = (TCG_TARGET_REG_BITS == 32 && is64 ? *args++ : 0);
+switch (type) {
+case TCG_TYPE_I32:
+datahi = 0;
+break;
+case TCG_TYPE_I64:
+datahi = (TCG_TARGET_REG_BITS == 32 ? *args++ : 0);
+break;
+default:
+g_assert_not_reached();
+}
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 25/29] tcg/i386: Mark Win64 call-saved vector regs as reserved
On 18/11/22 10:47, Richard Henderson wrote:
While we do not include these in tcg_target_reg_alloc_order,
and therefore they ought never be allocated, it seems safer
to mark them reserved as well.
Signed-off-by: Richard Henderson
---
tcg/i386/tcg-target.c.inc | 13 +
1 file changed, 13 insertions(+)
diff --git a/tcg/i386/tcg-target.c.inc b/tcg/i386/tcg-target.c.inc
index e38f08bd12..e04818eef6 100644
--- a/tcg/i386/tcg-target.c.inc
+++ b/tcg/i386/tcg-target.c.inc
@@ -4224,6 +4224,19 @@ static void tcg_target_init(TCGContext *s)
s->reserved_regs = 0;
tcg_regset_set_reg(s->reserved_regs, TCG_REG_CALL_STACK);
+#ifdef _WIN64
+/* These are call saved, and not we don't save them, so don't use them. */
s/not//?
Reviewed-by: Philippe Mathieu-Daudé
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM6);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM7);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM8);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM9);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM10);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM11);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM12);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM13);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM14);
+tcg_regset_set_reg(s->reserved_regs, TCG_REG_XMM15);
+#endif
}
typedef struct {
Re: [PATCH v5 14/20] hw/audio: explicitly set .requester_type for intel-hda
On 21/11/22 19:39, Peter Maydell wrote: On Fri, 11 Nov 2022 at 18:35, Alex Bennée wrote: This is simulating a bus master writing data back into system memory. Mark it as such. Signed-off-by: Alex Bennée --- hw/audio/intel-hda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) I wonder if stl_le_pci_dma() and friends should set the requester_id on the attrs that they are passed ? Very good point!
Re: [PATCH for-7.2] block-backend: avoid bdrv_unregister_buf() NULL pointer deref
On 21/11/22 22:19, Stefan Hajnoczi wrote:
bdrv_*() APIs expect a valid BlockDriverState. Calling them with bs=NULL
leads to undefined behavior.
Jonathan Cameron reported this following NULL pointer dereference when a
VM with a virtio-blk device and a memory-backend-file object is
terminated:
1. qemu_cleanup() closes all drives, setting blk->root to NULL
2. qemu_cleanup() calls user_creatable_cleanup(), which results in a RAM
block notifier callback because the memory-backend-file is destroyed.
3. blk_unregister_buf() is called by virtio-blk's BlockRamRegistrar
notifier callback and undefined behavior occurs.
Fixes: baf422684d73 ("virtio-blk: use BDRV_REQ_REGISTERED_BUF optimization
hint")
Co-authored-by: Jonathan Cameron
Signed-off-by: Stefan Hajnoczi
---
block/block-backend.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé
Re: UI layer threading and locking strategy; memory_region_snapshot_and_clear_dirty() races
Cc'ing more UI/display contributors. On 17/11/22 14:05, Peter Maydell wrote: On Tue, 1 Nov 2022 at 14:17, Peter Maydell wrote: Hi; I'm trying to find out what the UI layer's threading and locking strategy is, at least as far as it applies to display device models. Ping! :-) I'm still looking for information about this, and about what threads call_rcu() callbacks might be run on... thanks -- PMM Specifically: * is the device's GraphicHwOps::gfx_update method always called from one specific thread, or might it be called from any thread? * is that method called with any locks guaranteed held? (eg the iothread lock) * is the caller of the gfx_update method OK if an implementation of the method drops the iothread lock temporarily while it is executing? (my guess would be "no") * for a gfx_update_async = true device, what are the requirements on calling graphic_hw_update_done()? Does the caller need to hold any particular lock? Does the call need to be done from any particular thread? The background to this is that I'm looking again at the race condition involving the memory_region_snapshot_and_clear_dirty() function, as described here: https://lore.kernel.org/qemu-devel/CAFEAcA9odnPo2LPip295Uztri7JfoVnQbkJ=wn+k8dqneb_...@mail.gmail.com/T/#u Having worked through what is going on, as far as I can see: (1) in order to be sure that we have the right data to match the snapshotted dirty bitmap state, we must wait for all TCG vCPUs to leave their current TB (2) a vCPU might block waiting for the iothread lock mid-TB (3) therefore we cannot wait for the TCG vCPUs without dropping the iothread lock one way or another (4) but none of the callers expect that and various things break My tentative idea for a fix is a bit of an upheaval: * have the display devices set gfx_update_async = true * instead of doing everything synchronously in their gfx_update method, they do the initial setup and call an 'async' version of memory_region_snapshot_and_clear_dirty() * that async version of the function will do what it does today, but without trying to wait for TCG vCPUs * instead the caller arranges (via call_rcu(), probably) a callback that will happen once all the TCG CPUs have finished executing their current TB * that callback does the actual copy-from-guest-ram-to-display and then calls graphic_hw_update_done() This seems like an awful pain in the neck but I couldn't see anything better :-( Paolo: what (if any) guarantee does call_rcu() make about which thread the callback function gets executed on, and what locks are/are not held when it's called? (I haven't looked at the migration code's use of memory_global_after_dirty_log_sync() but I suspect it's similarly broken.) thanks -- PMM
Re: [PATCH v2 2/3] hw/mips/malta: Set PIIX4 IRQ routes in embedded bootloader
On 21/11/22 16:34, Bernhard Beschow wrote:
Am 27. Oktober 2022 20:47:19 UTC schrieb "Philippe Mathieu-Daudé"
:
Linux kernel expects the northbridge & southbridge chipsets
configured by the BIOS firmware. We emulate that by writing
a tiny bootloader code in write_bootloader().
Upon introduction in commit 5c2b87e34d ("PIIX4 support"),
the PIIX4 configuration space included values specific to
the Malta board.
Set the Malta-specific IRQ routing values in the embedded
bootloader, so the next commit can remove the Malta specific
bits from the PIIX4 PCI-ISA bridge and make it generic
(matching the real hardware).
Signed-off-by: Philippe Mathieu-Daudé
---
FIXME: Missing the nanoMIPS counter-part!
Who will be taking care of this? I have absolutely no clue how the
write_bootloader functions work, so I don't see how to fix it.
Oh actually I wrote that and tested it but context switched and forgot
about it... I'll look back when I get some time, probably around the
release.
Couldn't we just do it like in pegasos2_init() where the registers are
initialized by QEMU directly if there is no bootloader binary configured? I
could do that.
I rather mimic bootloaders... maybe a matter of taste?
Regards,
Phil.
Re: [PATCH for-8.0 20/29] tcg: Add INDEX_op_qemu_{ld,st}_i128
On 18/11/22 10:47, Richard Henderson wrote: Add opcodes for backend support for 128-bit memory operations. Signed-off-by: Richard Henderson --- include/tcg/tcg-opc.h| 8 + tcg/aarch64/tcg-target.h | 2 ++ tcg/arm/tcg-target.h | 2 ++ tcg/i386/tcg-target.h| 2 ++ tcg/loongarch64/tcg-target.h | 2 ++ tcg/mips/tcg-target.h| 2 ++ tcg/ppc/tcg-target.h | 2 ++ tcg/riscv/tcg-target.h | 2 ++ tcg/s390x/tcg-target.h | 2 ++ tcg/sparc64/tcg-target.h | 2 ++ tcg/tci/tcg-target.h | 2 ++ tcg/tcg-op.c | 67 tcg/tcg.c| 4 +++ tcg/README | 10 -- 14 files changed, 100 insertions(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 17/29] tcg/aarch64: Add have_lse, have_lse2
On 18/11/22 10:47, Richard Henderson wrote:
Notice when the host has additional atomic instructions.
The new variables will also be used in generated code.
Signed-off-by: Richard Henderson
---
tcg/aarch64/tcg-target.h | 3 +++
tcg/aarch64/tcg-target.c.inc | 10 ++
2 files changed, 13 insertions(+)
diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
index 001a71bbc0..cf5ee6f742 100644
--- a/tcg/aarch64/tcg-target.c.inc
+++ b/tcg/aarch64/tcg-target.c.inc
@@ -13,6 +13,8 @@
#include "../tcg-ldst.c.inc"
#include "../tcg-pool.c.inc"
#include "qemu/bitops.h"
+#include
This doesn't build on Darwin:
In file included from ../../tcg/tcg.c:426:
tcg/aarch64/tcg-target.c.inc:16:10: fatal error: 'asm/hwcap.h' file not
found
#include
^
In file included from ../../accel/tcg/cputlb.c:1656:
../../accel/tcg/ldst_atomicity.c.inc:269:21: warning: value size does
not match register size specified by the constraint and modifier
[-Wasm-operand-widths]
: "=&r"(r.u), "=&r"(fail) : "Q"(*p));
^
../../accel/tcg/ldst_atomicity.c.inc:266:22: note: use constraint
modifier "w"
asm("0: ldxp %0, %R0, %2\n\t"
^~
%w0
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
asm("0: ldxp %[t], %R[t], %[mem]\n\t"
^
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: unknown token in
expression
:1:15: note: instantiated into assembly here
0: ldxp x13, , [x9]
^
In file included from ../../accel/tcg/cputlb.c:1656:
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand
asm("0: ldxp %[t], %R[t], %[mem]\n\t"
^
:1:15: note: instantiated into assembly here
0: ldxp x13, , [x9]
^
In file included from ../../accel/tcg/cputlb.c:1656:
../../accel/tcg/ldst_atomicity.c.inc:903:32: error: unknown token in
expression
"bic %[t], %[t], %[m]\n\t"
^
:3:6: note: instantiated into assembly here
bic , ,
^
In file included from ../../accel/tcg/cputlb.c:1656:
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
asm("0: ldxp %[t], %R[t], %[mem]\n\t"
^
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
../../accel/tcg/ldst_atomicity.c.inc:902:9: error: invalid operand in
inline asm: '0: ldxp $2, ${2:R}, $0 bic $2, $2, $4 bic ${2:R}, ${2:R},
${4:R} orr $2, $2, $3 orr ${2:R}, ${2:R}, ${3:R} stxp ${1:w}, $2,
${2:R}, $0 cbnz ${1:w}, 0b'
fatal error: too many errors emitted, stopping now [-ferror-limit=]
Re: [PATCH for-8.0 17/29] tcg/aarch64: Add have_lse, have_lse2
On 22/11/22 00:10, Philippe Mathieu-Daudé wrote: On 18/11/22 10:47, Richard Henderson wrote: Notice when the host has additional atomic instructions. The new variables will also be used in generated code. Signed-off-by: Richard Henderson --- tcg/aarch64/tcg-target.h | 3 +++ tcg/aarch64/tcg-target.c.inc | 10 ++ 2 files changed, 13 insertions(+) diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc index 001a71bbc0..cf5ee6f742 100644 --- a/tcg/aarch64/tcg-target.c.inc +++ b/tcg/aarch64/tcg-target.c.inc @@ -13,6 +13,8 @@ #include "../tcg-ldst.c.inc" #include "../tcg-pool.c.inc" #include "qemu/bitops.h" +#include This doesn't build on Darwin: Project version: 7.1.91 C compiler for the host machine: clang (clang 14.0.0 "Apple clang version 14.0.0 (clang-1400.0.29.102)") C linker for the host machine: clang ld64 819.6 Host machine cpu family: aarch64 Host machine cpu: arm64 In file included from ../../tcg/tcg.c:426: tcg/aarch64/tcg-target.c.inc:16:10: fatal error: 'asm/hwcap.h' file not found #include ^ In file included from ../../accel/tcg/cputlb.c:1656: ../../accel/tcg/ldst_atomicity.c.inc:269:21: warning: value size does not match register size specified by the constraint and modifier [-Wasm-operand-widths] : "=&r"(r.u), "=&r"(fail) : "Q"(*p)); ^
Re: [PATCH for-8.0 15/29] include/qemu/int128: Add vector type to Int128Alias
On 18/11/22 10:47, Richard Henderson wrote: Adding a vector type will make it easier to handle i386 have_atomic16 via AVX. Signed-off-by: Richard Henderson --- include/qemu/int128.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 v3 12/45] tcg: Allocate TCGTemp pairs in host memory order
On 11/11/22 08:40, Richard Henderson wrote: Allocate the first of a pair at the lower address, and the second of a pair at the higher address. This will make it easier to find the beginning of the larger memory block. Signed-off-by: Richard Henderson --- tcg/tcg-internal.h | 4 ++-- tcg/tcg.c | 58 ++ 2 files changed, 30 insertions(+), 32 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 v3 14/45] tcg: Introduce tcg_type_size
On 11/11/22 08:40, Richard Henderson wrote:
Add a helper function for computing the size of a type.
Signed-off-by: Richard Henderson
---
include/tcg/tcg.h | 16
tcg/tcg.c | 26 --
2 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index f2da340bb9..8bcd60d0ed 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -319,6 +319,22 @@ typedef enum TCGType {
#endif
} TCGType;
+/**
+ * tcg_type_size
+ * @t: type
+ *
+ * Return the size of the type in bytes.
+ */
+static inline int tcg_type_size(TCGType t)
+{
+unsigned i = t;
+if (i >= TCG_TYPE_V64) {
+tcg_debug_assert(i < TCG_TYPE_COUNT);
+i -= TCG_TYPE_V64 - 1;
+}
+return 4 << i;
I'd feel safer if we assign TCG_TYPE_I32 .. TCG_TYPE_V256 in TCGType,
just in case.
+}
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 v3 15/45] tcg: Introduce TCGCallReturnKind and TCGCallArgumentKind
On 11/11/22 08:40, Richard Henderson wrote: Prepare to replace a bunch of separate ifdefs with a consistent way to describe the abi of a function call. Nitpicking, s/abi/ABI/ like in patch 40 "Fill in the parameters for the host ABI for Int128". Signed-off-by: Richard Henderson --- tcg/tcg-internal.h | 15 +++ 1 file changed, 15 insertions(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] cleanup: Tweak and re-run return_directly.cocci
On 22/11/22 09:58, Markus Armbruster wrote: Thomas Huth writes: On 21/11/2022 17.32, Markus Armbruster wrote: Philippe Mathieu-Daudé writes: On 21/11/22 15:36, Peter Maydell wrote: On Mon, 21 Nov 2022 at 14:03, Markus Armbruster wrote: Tweak the semantic patch to drop redundant parenthesis around the return expression. Coccinelle drops comments in hw/rdma/vmw/pvrdma_cmd.c; restored manually. Coccinelle messes up vmdk_co_create(), not sure why. Transformed manually. Line breaks in target/avr/cpu.h and hw/rdma/vmw/pvrdma_cmd.c tidied up manually. Whitespace in fuse_reply_iov() tidied up manually. checkpatch.pl complains "return of an errno should typically be -ve" two times for hw/9pfs/9p-synth.c. Preexisting, the patch merely makes it visible to checkpatch.pl. checkpatch.pl complains "return is not a function, parentheses are not required" three times for target/mips/tcg/dsp_helper.c. False positives. Signed-off-by: Markus Armbruster .../user/ase/msa/bit-count/test_msa_nloc_b.c | 9 +- .../user/ase/msa/bit-count/test_msa_nloc_d.c | 9 +- [snip long list of other mips test files] 328 files changed, 989 insertions(+), 2099 deletions(-) This patch seems to almost entirely be huge because of these mips test case files. Are they specific to QEMU or are they effectively a 3rd-party import that it doesn't make sense to make local changes to? They are imported and will unlikely be modified. Not obvious to me from git-log. Should I drop the changes to tests/tcg/mips/? I'd say yes. At least move them to a separate patch. Possible status of tests/tcg/mips/: 1. Imported, should not be modified Drop from the patch. 2. Not imported, should be modified 2a. To be reviewed separately from the remainder of the patch Split off. 2b. Likewise, but nobody will care to review, realistically Split off and merge anyway, or drop. I'd go for the latter. 2c. To be reviewed together with the remainder of the patch Keep as is. Which one is it? "1. Imported, should not be modified" please :)
Re: [PATCH] cleanup: Tweak and re-run return_directly.cocci
On 21/11/22 17:42, Max Filippov wrote: On Mon, Nov 21, 2022 at 6:01 AM Markus Armbruster wrote: .../xtensa/core-dsp3400/xtensa-modules.c.inc | 136 +- target/xtensa/core-lx106/xtensa-modules.c.inc | 16 +-- These files are generated and were imported from xtensa configuration overlays, they're not supposed to be changed. Tools can get the repository file list using 'git ls-files', which itself support file pattern exclusion [*]. We can create i.e. 'scripts/imported-files.txt' with: linux-headers/ target/hexagon/imported/ target/xtensa/core* tests/tcg/mips/user/ Then use 'git ls-files --exclude-from=scripts/imported-files.txt' ... [*] https://git-scm.com/docs/git-ls-files#Documentation/git-ls-files.txt---exclude-fromltfilegt
Re: [PATCH v2 2/2] block/vmdk: Simplify vmdk_co_create() to return directly
On 22/11/22 14:49, Markus Armbruster wrote: Cc: Fam Zheng Cc: Kevin Wolf Cc: Hanna Reitz Cc: qemu-bl...@nongnu.org Signed-off-by: Markus Armbruster --- block/vmdk.c | 28 +++- 1 file changed, 11 insertions(+), 17 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2 1/2] cleanup: Tweak and re-run return_directly.cocci
On 22/11/22 14:49, Markus Armbruster wrote: Tweak the semantic patch to drop redundant parenthesis around the return expression. Coccinelle drops a comment in hw/rdma/vmw/pvrdma_cmd.c; restored manually. Coccinelle messes up vmdk_co_create(), not sure why. Change dropped, will be done manually in the next commit. Line breaks in target/avr/cpu.h and hw/rdma/vmw/pvrdma_cmd.c tidied up manually. Whitespace in tools/virtiofsd/fuse_lowlevel.c tidied up manually. checkpatch.pl complains "return of an errno should typically be -ve" two times for hw/9pfs/9p-synth.c. Preexisting, the patch merely makes it visible to checkpatch.pl. Signed-off-by: Markus Armbruster --- scripts/coccinelle/return_directly.cocci | 5 +-- include/hw/pci/pci.h | 7 +-- target/avr/cpu.h | 4 +- hw/9pfs/9p-synth.c | 14 ++ hw/char/sifive_uart.c| 4 +- hw/ppc/ppc4xx_sdram.c| 5 +-- hw/rdma/vmw/pvrdma_cmd.c | 57 +--- hw/virtio/vhost-user.c | 6 +-- migration/dirtyrate.c| 10 + migration/tls.c | 6 +-- replay/replay-time.c | 5 +-- semihosting/console.c| 4 +- softmmu/memory.c | 11 + softmmu/physmem.c| 9 +--- target/loongarch/cpu.c | 4 +- target/mips/tcg/dsp_helper.c | 15 ++- target/riscv/debug.c | 6 +-- target/riscv/vector_helper.c | 28 +++- tests/bench/benchmark-crypto-akcipher.c | 6 +-- tests/qtest/erst-test.c | 5 +-- tests/qtest/hexloader-test.c | 6 +-- tests/qtest/pvpanic-pci-test.c | 6 +-- tests/qtest/pvpanic-test.c | 6 +-- tests/qtest/test-filter-mirror.c | 6 +-- tests/qtest/virtio-ccw-test.c| 6 +-- tests/tcg/multiarch/sha512.c | 9 +--- tools/virtiofsd/fuse_lowlevel.c | 24 +++--- 27 files changed, 70 insertions(+), 204 deletions(-) diff --git a/scripts/coccinelle/return_directly.cocci b/scripts/coccinelle/return_directly.cocci index 4cf50e75ea..6cb1b3c99a 100644 --- a/scripts/coccinelle/return_directly.cocci +++ b/scripts/coccinelle/return_directly.cocci @@ -11,9 +11,8 @@ identifier F; -T VAR; ... when != VAR --VAR = -+return - E; +-VAR = (E); -return VAR; ++return E; ... when != VAR } Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH-for-7.2] vhost-vdpa: skip TPM CRB memory section
On 22/11/22 15:53, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
851d6d1a0f ("vfio/common: remove spurious tpm-crb-cmd misalignment
warning") removed the warning on vfio_listener_region_add() path.
An error is reported for vhost-vdpa case:
qemu-kvm: vhost_vdpa_listener_region_add received unaligned region
Skip the CRB device.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=2141965
Signed-off-by: Marc-André Lureau
---
hw/virtio/vhost-vdpa.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 7468e44b87..9d7206e4b8 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -19,6 +19,7 @@
#include "hw/virtio/virtio-net.h"
#include "hw/virtio/vhost-shadow-virtqueue.h"
#include "hw/virtio/vhost-vdpa.h"
+#include "sysemu/tpm.h"
#include "exec/address-spaces.h"
#include "migration/blocker.h"
#include "qemu/cutils.h"
@@ -46,6 +47,11 @@ static bool
vhost_vdpa_listener_skipped_section(MemoryRegionSection *section,
{
Int128 llend;
+if (TPM_IS_CRB(section->mr->owner)) {
+/* The CRB command buffer has its base address unaligned. */
+return true;
+}
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 v3 20/45] accel/tcg/plugin: Avoid duplicate copy in copy_call
On 11/11/22 08:40, Richard Henderson wrote: We copied all of the arguments in copy_op_nocheck. We only need to replace the one argument that we change. Signed-off-by: Richard Henderson --- accel/tcg/plugin-gen.c | 2 -- 1 file changed, 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 v3 21/45] accel/tcg/plugin: Use copy_op in append_{udata, mem}_cb
On 11/11/22 08:40, Richard Henderson wrote: Better to re-use the existing function for copying ops. Signed-off-by: Richard Henderson --- accel/tcg/plugin-gen.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 v3 22/45] tci: MAX_OPC_PARAM_IARGS is no longer used
On 11/11/22 08:40, Richard Henderson wrote:
Signed-off-by: Richard Henderson
---
tcg/tci.c| 1 -
tcg/tci/tcg-target.c.inc | 4
2 files changed, 5 deletions(-)
Since commit 7b7d8b2d9a ("tcg/tci: Use ffi for calls")?
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for-8.0 v3 24/45] tcg: Use output_pref wrapper function
On 11/11/22 08:40, Richard Henderson wrote: We will shortly have the possibility of more that two outputs, though only for calls (for which preferences are moot). Avoid direct references to op->output_pref[] when possible. Signed-off-by: Richard Henderson --- include/tcg/tcg.h | 5 + tcg/tcg.c | 34 ++ 2 files changed, 23 insertions(+), 16 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
[PATCH 0/3] tcg: Move ffi_cif pointer into TCGHelperInfo (splitted)
Hi Richard, Your "Move ffi_cif pointer into TCGHelperInfo" patch was not obvious (to me) at first, so I split it in 3 trivial patches. Philippe Mathieu-Daudé (2): tcg: Convert typecode_to_ffi from array to function tcg: Factor init_ffi_layouts() out of tcg_context_init() Richard Henderson (1): tcg: Move ffi_cif pointer into TCGHelperInfo tcg/tcg-internal.h | 7 +++ tcg/tcg.c | 125 + 2 files changed, 78 insertions(+), 54 deletions(-) -- 2.38.1
[PATCH 3/3] tcg: Move ffi_cif pointer into TCGHelperInfo
From: Richard Henderson
Instead of requiring a separate hash table lookup,
put a pointer to the CIF into TCGHelperInfo.
Signed-off-by: Richard Henderson
Message-Id: <2022074101.2069454-27-richard.hender...@linaro.org>
[PMD: Split from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé
---
tcg/tcg-internal.h | 7 +++
tcg/tcg.c | 26 ++
2 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/tcg/tcg-internal.h b/tcg/tcg-internal.h
index c7e87e193d..6e50aeba3a 100644
--- a/tcg/tcg-internal.h
+++ b/tcg/tcg-internal.h
@@ -25,6 +25,10 @@
#ifndef TCG_INTERNAL_H
#define TCG_INTERNAL_H
+#ifdef CONFIG_TCG_INTERPRETER
+#include
+#endif
+
#define TCG_HIGHWATER 1024
/*
@@ -57,6 +61,9 @@ typedef struct TCGCallArgumentLoc {
typedef struct TCGHelperInfo {
void *func;
const char *name;
+#ifdef CONFIG_TCG_INTERPRETER
+ffi_cif *cif;
+#endif
unsigned typemask : 32;
unsigned flags : 8;
unsigned nr_in : 8;
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 9b24b4d863..d6a3036412 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -552,8 +552,6 @@ static TCGHelperInfo all_helpers[] = {
static GHashTable *helper_table;
#ifdef CONFIG_TCG_INTERPRETER
-static GHashTable *ffi_table;
-
static ffi_type *typecode_to_ffi(int argmask)
{
switch (argmask) {
@@ -576,9 +574,11 @@ static ffi_type *typecode_to_ffi(int argmask)
static void init_ffi_layouts(void)
{
/* g_direct_hash/equal for direct comparisons on uint32_t. */
-ffi_table = g_hash_table_new(NULL, NULL);
+GHashTable *ffi_table = g_hash_table_new(NULL, NULL);
+
for (int i = 0; i < ARRAY_SIZE(all_helpers); ++i) {
-uint32_t typemask = all_helpers[i].typemask;
+TCGHelperInfo *info = &all_helpers[i];
+unsigned typemask = info->typemask;
gpointer hash = (gpointer)(uintptr_t)typemask;
struct {
ffi_cif cif;
@@ -586,8 +586,11 @@ static void init_ffi_layouts(void)
} *ca;
ffi_status status;
int nargs;
+ffi_cif *cif;
-if (g_hash_table_lookup(ffi_table, hash)) {
+cif = g_hash_table_lookup(ffi_table, hash);
+if (cif) {
+info->cif = cif;
continue;
}
@@ -611,8 +614,12 @@ static void init_ffi_layouts(void)
ca->cif.rtype, ca->cif.arg_types);
assert(status == FFI_OK);
-g_hash_table_insert(ffi_table, hash, (gpointer)&ca->cif);
+cif = &ca->cif;
+info->cif = cif;
+g_hash_table_insert(ffi_table, hash, (gpointer)cif);
}
+
+g_hash_table_destroy(ffi_table);
}
#endif /* CONFIG_TCG_INTERPRETER */
@@ -4413,12 +4420,7 @@ static void tcg_reg_alloc_call(TCGContext *s, TCGOp *op)
}
#ifdef CONFIG_TCG_INTERPRETER
-{
-gpointer hash = (gpointer)(uintptr_t)info->typemask;
-ffi_cif *cif = g_hash_table_lookup(ffi_table, hash);
-assert(cif != NULL);
-tcg_out_call(s, tcg_call_func(op), cif);
-}
+tcg_out_call(s, tcg_call_func(op), info->cif);
#else
tcg_out_call(s, tcg_call_func(op));
#endif
--
2.38.1
[PATCH 1/3] tcg: Convert typecode_to_ffi from array to function
In the unlikely case of invalid typecode mask, the function
will abort instead of returning a NULL pointer.
Signed-off-by: Richard Henderson
Message-Id: <2022074101.2069454-27-richard.hender...@linaro.org>
[PMD: Split from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé
---
tcg/tcg.c | 30 --
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index cabc397a38..8aa6fc9a25 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -554,14 +554,24 @@ static GHashTable *helper_table;
#ifdef CONFIG_TCG_INTERPRETER
static GHashTable *ffi_table;
-static ffi_type * const typecode_to_ffi[8] = {
-[dh_typecode_void] = &ffi_type_void,
-[dh_typecode_i32] = &ffi_type_uint32,
-[dh_typecode_s32] = &ffi_type_sint32,
-[dh_typecode_i64] = &ffi_type_uint64,
-[dh_typecode_s64] = &ffi_type_sint64,
-[dh_typecode_ptr] = &ffi_type_pointer,
-};
+static ffi_type *typecode_to_ffi(int argmask)
+{
+switch (argmask) {
+case dh_typecode_void:
+return &ffi_type_void;
+case dh_typecode_i32:
+return &ffi_type_uint32;
+case dh_typecode_s32:
+return &ffi_type_sint32;
+case dh_typecode_i64:
+return &ffi_type_uint64;
+case dh_typecode_s64:
+return &ffi_type_sint64;
+case dh_typecode_ptr:
+return &ffi_type_pointer;
+}
+g_assert_not_reached();
+}
#endif
typedef struct TCGCumulativeArgs {
@@ -764,14 +774,14 @@ static void tcg_context_init(unsigned max_cpus)
nargs = DIV_ROUND_UP(nargs, 3);
ca = g_malloc0(sizeof(*ca) + nargs * sizeof(ffi_type *));
-ca->cif.rtype = typecode_to_ffi[typemask & 7];
+ca->cif.rtype = typecode_to_ffi(typemask & 7);
ca->cif.nargs = nargs;
if (nargs != 0) {
ca->cif.arg_types = ca->args;
for (int j = 0; j < nargs; ++j) {
int typecode = extract32(typemask, (j + 1) * 3, 3);
-ca->args[j] = typecode_to_ffi[typecode];
+ca->args[j] = typecode_to_ffi(typecode);
}
}
--
2.38.1
[PATCH 2/3] tcg: Factor init_ffi_layouts() out of tcg_context_init()
Signed-off-by: Richard Henderson
Message-Id: <2022074101.2069454-27-richard.hender...@linaro.org>
[PMD: Split from bigger patch]
Signed-off-by: Philippe Mathieu-Daudé
---
tcg/tcg.c | 83 +--
1 file changed, 44 insertions(+), 39 deletions(-)
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 8aa6fc9a25..9b24b4d863 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -572,7 +572,49 @@ static ffi_type *typecode_to_ffi(int argmask)
}
g_assert_not_reached();
}
-#endif
+
+static void init_ffi_layouts(void)
+{
+/* g_direct_hash/equal for direct comparisons on uint32_t. */
+ffi_table = g_hash_table_new(NULL, NULL);
+for (int i = 0; i < ARRAY_SIZE(all_helpers); ++i) {
+uint32_t typemask = all_helpers[i].typemask;
+gpointer hash = (gpointer)(uintptr_t)typemask;
+struct {
+ffi_cif cif;
+ffi_type *args[];
+} *ca;
+ffi_status status;
+int nargs;
+
+if (g_hash_table_lookup(ffi_table, hash)) {
+continue;
+}
+
+/* Ignoring the return type, find the last non-zero field. */
+nargs = 32 - clz32(typemask >> 3);
+nargs = DIV_ROUND_UP(nargs, 3);
+
+ca = g_malloc0(sizeof(*ca) + nargs * sizeof(ffi_type *));
+ca->cif.rtype = typecode_to_ffi(typemask & 7);
+ca->cif.nargs = nargs;
+
+if (nargs != 0) {
+ca->cif.arg_types = ca->args;
+for (int j = 0; j < nargs; ++j) {
+int typecode = extract32(typemask, (j + 1) * 3, 3);
+ca->args[j] = typecode_to_ffi(typecode);
+}
+}
+
+status = ffi_prep_cif(&ca->cif, FFI_DEFAULT_ABI, nargs,
+ ca->cif.rtype, ca->cif.arg_types);
+assert(status == FFI_OK);
+
+g_hash_table_insert(ffi_table, hash, (gpointer)&ca->cif);
+}
+}
+#endif /* CONFIG_TCG_INTERPRETER */
typedef struct TCGCumulativeArgs {
int arg_idx;/* tcg_gen_callN args[] */
@@ -753,44 +795,7 @@ static void tcg_context_init(unsigned max_cpus)
}
#ifdef CONFIG_TCG_INTERPRETER
-/* g_direct_hash/equal for direct comparisons on uint32_t. */
-ffi_table = g_hash_table_new(NULL, NULL);
-for (i = 0; i < ARRAY_SIZE(all_helpers); ++i) {
-struct {
-ffi_cif cif;
-ffi_type *args[];
-} *ca;
-uint32_t typemask = all_helpers[i].typemask;
-gpointer hash = (gpointer)(uintptr_t)typemask;
-ffi_status status;
-int nargs;
-
-if (g_hash_table_lookup(ffi_table, hash)) {
-continue;
-}
-
-/* Ignoring the return type, find the last non-zero field. */
-nargs = 32 - clz32(typemask >> 3);
-nargs = DIV_ROUND_UP(nargs, 3);
-
-ca = g_malloc0(sizeof(*ca) + nargs * sizeof(ffi_type *));
-ca->cif.rtype = typecode_to_ffi(typemask & 7);
-ca->cif.nargs = nargs;
-
-if (nargs != 0) {
-ca->cif.arg_types = ca->args;
-for (int j = 0; j < nargs; ++j) {
-int typecode = extract32(typemask, (j + 1) * 3, 3);
-ca->args[j] = typecode_to_ffi(typecode);
-}
-}
-
-status = ffi_prep_cif(&ca->cif, FFI_DEFAULT_ABI, nargs,
- ca->cif.rtype, ca->cif.arg_types);
-assert(status == FFI_OK);
-
-g_hash_table_insert(ffi_table, hash, (gpointer)&ca->cif);
-}
+init_ffi_layouts();
#endif
tcg_target_init(s);
--
2.38.1
Re: [PATCH for-8.0 v3 14/45] tcg: Introduce tcg_type_size
On 22/11/22 17:54, Richard Henderson wrote:
On 11/22/22 03:30, Philippe Mathieu-Daudé wrote:
On 11/11/22 08:40, Richard Henderson wrote:
Add a helper function for computing the size of a type.
Signed-off-by: Richard Henderson
---
include/tcg/tcg.h | 16
tcg/tcg.c | 26 --
2 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index f2da340bb9..8bcd60d0ed 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -319,6 +319,22 @@ typedef enum TCGType {
#endif
} TCGType;
+/**
+ * tcg_type_size
+ * @t: type
+ *
+ * Return the size of the type in bytes.
+ */
+static inline int tcg_type_size(TCGType t)
+{
+ unsigned i = t;
+ if (i >= TCG_TYPE_V64) {
+ tcg_debug_assert(i < TCG_TYPE_COUNT);
+ i -= TCG_TYPE_V64 - 1;
+ }
+ return 4 << i;
I'd feel safer if we assign TCG_TYPE_I32 .. TCG_TYPE_V256 in TCGType,
just in case.
What do you mean?
-- >8 --
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
@@ -289,8 +289,8 @@ typedef struct TCGPool {
typedef enum TCGType {
-TCG_TYPE_I32,
-TCG_TYPE_I64,
+TCG_TYPE_I32 = 0,
+TCG_TYPE_I64 = 1,
-TCG_TYPE_V64,
-TCG_TYPE_V128,
-TCG_TYPE_V256,
+TCG_TYPE_V64 = 2,
+TCG_TYPE_V128 = 3,
+TCG_TYPE_V256 = 4,
---
Re: [PATCH for-8.0 15/29] include/qemu/int128: Add vector type to Int128Alias
On 18/11/22 10:47, Richard Henderson wrote:
Adding a vector type will make it easier to handle i386
have_atomic16 via AVX.
Signed-off-by: Richard Henderson
---
include/qemu/int128.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index f62a46b48c..f29f90e6f4 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -479,16 +479,16 @@ static inline void bswap128s(Int128 *s)
/*
* When compiler supports a 128-bit type, define a combination of
* a possible structure and the native types. Ease parameter passing
- * via use of the transparent union extension.
+ * via use of the transparent union extension. Provide a vector type
+ * for use in atomicity on some hosts.
*/
-#ifdef CONFIG_INT128
typedef union {
Int128 s;
+uint64_t v __attribute__((vector_size(16)));
+#ifdef CONFIG_INT128
__int128_t i;
__uint128_t u;
-} Int128Alias __attribute__((transparent_union));
-#else
-typedef Int128 Int128Alias;
#endif /* CONFIG_INT128 */
+} Int128Alias __attribute__((transparent_union));
#endif /* INT128_H */
This triggers a warning with GCC:
include/qemu/int128.h:487:14: warning: alignment of field 'v' (128 bits)
does not match the alignment of the first field in transparent union;
transparent_union attribute ignored [-Wignored-attributes]
uint64_t v __attribute__((vector_size(16)));
^
include/qemu/int128.h:486:12: note: alignment of first field is 64 bits
Int128 s;
^
Meson:
Project version: 7.1.91
C compiler for the host machine: gcc-12 (gcc 12.2.0 "gcc-12 (Homebrew
GCC 12.2.0) 12.2.0")
C linker for the host machine: gcc-12 ld64 819.6
Host machine cpu family: aarch64
Host machine cpu: arm64
Re: [PATCH for-8.0 15/29] include/qemu/int128: Add vector type to Int128Alias
On 22/11/22 19:21, Philippe Mathieu-Daudé wrote:
On 18/11/22 10:47, Richard Henderson wrote:
Adding a vector type will make it easier to handle i386
have_atomic16 via AVX.
Signed-off-by: Richard Henderson
---
include/qemu/int128.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index f62a46b48c..f29f90e6f4 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -479,16 +479,16 @@ static inline void bswap128s(Int128 *s)
/*
* When compiler supports a 128-bit type, define a combination of
* a possible structure and the native types. Ease parameter passing
- * via use of the transparent union extension.
+ * via use of the transparent union extension. Provide a vector type
+ * for use in atomicity on some hosts.
*/
-#ifdef CONFIG_INT128
typedef union {
Int128 s;
+ uint64_t v __attribute__((vector_size(16)));
+#ifdef CONFIG_INT128
__int128_t i;
__uint128_t u;
-} Int128Alias __attribute__((transparent_union));
-#else
-typedef Int128 Int128Alias;
#endif /* CONFIG_INT128 */
+} Int128Alias __attribute__((transparent_union));
#endif /* INT128_H */
This triggers a warning with GCC:
Ah no, looking closer, even configured as ''--cc=gcc-12 --host-cc=gcc-12
--cxx=/bin/false', Clang got selected for ObjC, and this warning comes
from it:
Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
In file included from ../../ui/cocoa.m:36:
In file included from include/sysemu/sysemu.h:5:
In file included from include/qemu/timer.h:4:
In file included from include/qemu/bitops.h:16:
In file included from include/qemu/host-utils.h:35:
include/qemu/int128.h:487:14: warning: alignment of field 'v' (128 bits)
does not match the alignment of the first field in transparent union;
transparent_union attribute ignored [-Wignored-attributes]
uint64_t v __attribute__((vector_size(16)));
^
include/qemu/int128.h:486:12: note: alignment of first field is 64 bits
Int128 s;
^
Meson:
Project version: 7.1.91
C compiler for the host machine: gcc-12 (gcc 12.2.0 "gcc-12 (Homebrew
GCC 12.2.0) 12.2.0")
C linker for the host machine: gcc-12 ld64 819.6
Host machine cpu family: aarch64
Host machine cpu: arm64
Objective-C compiler for the host machine: clang (clang 14.0.0)
Objective-C linker for the host machine: clang ld64 819.6
Regards,
Phil.
Re: [PATCH v2 1/7] qemu/main-loop: Introduce QEMU_IOTHREAD_LOCK_GUARD, WITH_QEMU_IOTHREAD_LOCK
On 22/11/22 21:57, Richard Henderson wrote: Create a couple of wrappers for locking/unlocking the iothread lock. Signed-off-by: Richard Henderson --- include/qemu/main-loop.h | 39 +++ 1 file changed, 39 insertions(+) +#define QEMU_IOTHREAD_LOCK_GUARD() \ +g_auto(IOThreadLockAuto) _iothread_lock_auto\ += qemu_iothread_auto_lock(__FILE__, __LINE__) \ + +#define WITH_QEMU_IOTHREAD_LOCK() \ +for (QEMU_IOTHREAD_LOCK_GUARD();\ + _iothread_lock_auto.iterate; \ + _iothread_lock_auto.iterate = false) Nice, thanks! Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v2] hw/loongarch: Add cfi01 pflash device
On 23/11/22 02:23, Xiaojuan Yang wrote:
Add cfi01 pflash device for LoongArch virt machine
Signed-off-by: Xiaojuan Yang
---
hw/loongarch/Kconfig| 1 +
hw/loongarch/acpi-build.c | 18 +
hw/loongarch/virt.c | 80 -
include/hw/loongarch/virt.h | 5 +++
4 files changed, 103 insertions(+), 1 deletion(-)
static void fdt_add_rtc_node(LoongArchMachineState *lams)
{
@@ -593,9 +661,17 @@ static void loongarch_firmware_init(LoongArchMachineState
*lams)
{
char *filename = MACHINE(lams)->firmware;
char *bios_name = NULL;
-int bios_size;
+int bios_size, i;
lams->bios_loaded = false;
+/* Map legacy -drive if=pflash to machine properties */
+for (i = 0; i < ARRAY_SIZE(lams->flash); i++) {
+pflash_cfi01_legacy_drive(lams->flash[i],
+ drive_get(IF_PFLASH, 0, i));
My understanding is we shouldn't use pflash_cfi01_legacy_drive()
anymore, besides I don't think you requires it (for the machine
property).
(Cc'ing Markus for commit 2d731dbd5e).
This is unfortunate we let the sbsa-ref and riscv-virt machines
use it.
+}
+
+virt_flash_map(lams, get_system_memory());
+
if (filename) {
bios_name = qemu_find_file(QEMU_FILE_TYPE_BIOS, filename);
if (!bios_name) {
@@ -779,6 +855,7 @@ static void loongarch_init(MachineState *machine)
loongarch_direct_kernel_boot(lams);
}
}
+fdt_add_flash_node(lams);
/* register reset function */
for (i = 0; i < machine->smp.cpus; i++) {
lacpu = LOONGARCH_CPU(qemu_get_cpu(i));
@@ -838,6 +915,7 @@ static void loongarch_machine_initfn(Object *obj)
lams->acpi = ON_OFF_AUTO_AUTO;
lams->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
lams->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
+virt_flash_create(lams);
}
static bool memhp_type_supported(DeviceState *dev)
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 45c383f5a7..94afc92850 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -12,6 +12,7 @@
#include "hw/boards.h"
#include "qemu/queue.h"
#include "hw/intc/loongarch_ipi.h"
+#include "hw/block/flash.h"
#define LOONGARCH_MAX_VCPUS 4
@@ -20,6 +21,9 @@
#define VIRT_FWCFG_BASE 0x1e02UL
#define VIRT_BIOS_BASE 0x1c00UL
#define VIRT_BIOS_SIZE (4 * MiB)
+#define VIRT_FLASH_SECTOR_SIZE (128 * KiB)
+#define VIRT_FLASH0_BASE(VIRT_BIOS_BASE + VIRT_BIOS_SIZE)
Do you really want the flash base addr to depend of the ROM size?
It could be safer/simpler to start with a fixed address, leaving
room for a bigger ROM if you think you might have to use one.
+#define VIRT_FLASH0_SIZE(4 * MiB)
The '0' index in the name is not really useful / needed.
Note, if you provide addr/size to build_flash_aml(), these
definitions can be restricted to hw/loongarch/virt.c.
#define VIRT_LOWMEM_BASE0
#define VIRT_LOWMEM_SIZE0x1000
@@ -48,6 +52,7 @@ struct LoongArchMachineState {
int fdt_size;
DeviceState *platform_bus_dev;
PCIBus *pci_bus;
+PFlashCFI01 *flash[1];
The array is not really needed.
};
#define TYPE_LOONGARCH_MACHINE MACHINE_TYPE_NAME("virt")
Re: [RFC PATCH] tests/avocado: use new rootfs for orangepi test
On 18/11/22 12:33, Alex Bennée wrote:
The old URL wasn't stable. I suspect the current URL will only be
stable for a few months so maybe we need another strategy for hosting
rootfs snapshots?
Signed-off-by: Alex Bennée
---
tests/avocado/boot_linux_console.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/avocado/boot_linux_console.py
b/tests/avocado/boot_linux_console.py
index 4c9d551f47..5a2923c423 100644
--- a/tests/avocado/boot_linux_console.py
+++ b/tests/avocado/boot_linux_console.py
@@ -793,8 +793,8 @@ def test_arm_orangepi_sd(self):
dtb_path =
'/usr/lib/linux-image-current-sunxi/sun8i-h3-orangepi-pc.dtb'
dtb_path = self.extract_from_deb(deb_path, dtb_path)
rootfs_url = ('http://storage.kernelci.org/images/rootfs/buildroot/'
- 'kci-2019.02/armel/base/rootfs.ext2.xz')
-rootfs_hash = '692510cb625efda31640d1de0a8d60e26040f061'
+ 'buildroot-baseline/20221116.0/armel/rootfs.ext2.xz')
+rootfs_hash = 'fae32f337c7b87547b10f42599acf109da8b6d9a'
If Avocado doesn't find an artifact in its local cache, it will fetch it
from the URL.
The cache might be populated with artifacts previously downloaded, but
their URL is not valid anymore (my case for many tests).
We can also add artifacts manually, see [1].
I'd rather keep pre-existing tests if possible, to test older (kernel /
user-space) images. We don't need to run all the tests all the time:
tests can be filtered by tags (see [2]).
My preference here is to refactor this test, adding the "kci-2019.02"
and "baseline-20221116.0" releases. I can prepare the patch if you /
Thomas don't object.
Regards,
Phil.
[1]
https://avocado-framework.readthedocs.io/en/latest/guides/user/chapters/assets.html#registering-assets
[2]
https://avocado-framework.readthedocs.io/en/latest/guides/user/chapters/tags.html#filtering-tests-by-tags
