Re: [PATCH 00/33] hw/cpu/arm: Remove one use of qemu_get_cpu() in A7/A15 MPCore priv
On 1/9/24 22:09, Philippe Mathieu-Daudé wrote:
On 9/1/24 22:07, Philippe Mathieu-Daudé wrote:
Hi Cédric,
On 9/1/24 19:06, Cédric Le Goater wrote:
On 1/9/24 18:40, Fabiano Rosas wrote:
Cédric Le Goater writes:
On 1/3/24 20:53, Fabiano Rosas wrote:
Philippe Mathieu-Daudé writes:
+Peter/Fabiano
On 2/1/24 17:41, Cédric Le Goater wrote:
On 1/2/24 17:15, Philippe Mathieu-Daudé wrote:
Hi Cédric,
On 2/1/24 15:55, Cédric Le Goater wrote:
On 12/12/23 17:29, Philippe Mathieu-Daudé wrote:
Hi,
When a MPCore cluster is used, the Cortex-A cores belong the the
cluster container, not to the board/soc layer. This series move
the creation of vCPUs to the MPCore private container.
Doing so we consolidate the QOM model, moving common code in a
central place (abstract MPCore parent).
Changing the QOM hierarchy has an impact on the state of the machine
and some fixups are then required to maintain migration compatibility.
This can become a real headache for KVM machines like virt for which
migration compatibility is a feature, less for emulated ones.
All changes are either moving properties (which are not migrated)
or moving non-migrated QOM members (i.e. pointers of ARMCPU, which
is still migrated elsewhere). So I don't see any obvious migration
problem, but I might be missing something, so I Cc'ed Juan :>
FWIW, I didn't spot anything problematic either.
I've ran this through my migration compatibility series [1] and it
doesn't regress aarch64 migration from/to 8.2. The tests use '-M
virt -cpu max', so the cortex-a7 and cortex-a15 are not covered. I don't
think we even support migration of anything non-KVM on arm.
it happens we do.
Oh, sorry, I didn't mean TCG here. Probably meant to say something like
non-KVM-capable cpus, as in 32-bit. Nevermind.
Theoretically, we should be able to migrate to a TCG guest. Well, this
worked in the past for PPC. When I was doing more KVM related changes,
this was very useful for dev. Also, some machines are partially emulated.
Anyhow I agree this is not a strong requirement and we often break it.
Let's focus on KVM only.
No no, we want the same for TCG.
1- https://gitlab.com/farosas/qemu/-/jobs/5853599533
yes it depends on the QOM hierarchy and virt seems immune to the changes.
Good.
However, changing the QOM topology clearly breaks migration compat,
Well, "clearly" is relative =) You've mentioned pseries and aspeed
already, do you have a pointer to one of those cases were we broke
migration
Regarding pseries, migration compat broke because of 5bc8d26de20c
("spapr: allocate the ICPState object from under sPAPRCPUCore") which
is similar to the changes proposed by this series, it impacts the QOM
hierarchy. Here is the workaround/fix from Greg : 46f7afa37096
("spapr: fix migration of ICPState objects from/to older QEMU") which
is quite an headache and this turned out to raise another problem some
months ago ... :/ That's why I sent [1] to prepare removal of old
machines and workarounds becoming a burden.
Regarding aspeed, this series breaks compat.
Can you write down the steps to reproduce please? I'll debug it.
Also, have you figured (bisecting) which patch start to break?
Sorry I didn't. I wished I had more time for that.
Thanks,
C.
RE: FW: [PATCH] vhost-user: add VIRTIO_F_IN_ORDER and VIRTIO_F_NOTIFICATION_DATA feature
Hi, Michael and Jason
Do you have any other comments?
Is there a schedule for merge the patch into the community?
Thank you
Wentao
-Original Message-
From: Wentao Jia
Sent: Tuesday, January 2, 2024 1:57 PM
To: qemu-devel@nongnu.org
Cc: 'm...@redhat.com' ; Rick Zhong
; 'Jason Wang'
Subject: RE: FW: [PATCH] vhost-user: add VIRTIO_F_IN_ORDER and
VIRTIO_F_NOTIFICATION_DATA feature
---
hw/net/vhost_net.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index
e8e1661646..211ca859a6 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -76,6 +76,8 @@ static const int user_feature_bits[] = {
VIRTIO_F_IOMMU_PLATFORM,
VIRTIO_F_RING_PACKED,
VIRTIO_F_RING_RESET,
+VIRTIO_F_IN_ORDER,
+VIRTIO_F_NOTIFICATION_DATA,
VIRTIO_NET_F_RSS,
VIRTIO_NET_F_HASH_REPORT,
VIRTIO_NET_F_GUEST_USO4,
--
-Original Message-
From: Wentao Jia
Sent: Tuesday, January 2, 2024 1:38 PM
To: Jason Wang
Cc: m...@redhat.com; Rick Zhong
Subject: RE: FW: [PATCH] vhost-user: add VIRTIO_F_IN_ORDER and
VIRTIO_F_NOTIFICATION_DATA feature
Hi, Jason
It is good just change feature bits, I will commit a new patch, thanks
Wentao Jia
-Original Message-
From: Jason Wang
Sent: Tuesday, January 2, 2024 11:24 AM
To: Wentao Jia
Cc: m...@redhat.com; Rick Zhong
Subject: Re: FW: [PATCH] vhost-user: add VIRTIO_F_IN_ORDER and
VIRTIO_F_NOTIFICATION_DATA feature
On Tue, Jan 2, 2024 at 10:26 AM Wentao Jia wrote:
>
> Hi, Michael and Jason
>
>
>
> please review the patch at your convenience, thank you
>
> vhost-user: add VIRTIO_F_IN_ORDER and VIRTIO_F_NOTIFICATION_DATA
> feature - Patchwork (kernel.org)
>
>
>
> Wentao Jia
>
>
>
> From: Wentao Jia
> Sent: Friday, December 1, 2023 6:11 PM
> To: qemu-devel@nongnu.org
> Subject: [PATCH] vhost-user: add VIRTIO_F_IN_ORDER and
> VIRTIO_F_NOTIFICATION_DATA feature
>
>
>
> VIRTIO_F_IN_ORDER and VIRTIO_F_NOTIFICATION_DATA feature are important
> feature
>
> for dpdk vdpa packets transmitting performance, add the 2 features at
> vhost-user
>
> front-end to negotiation with backend.
>
>
>
> Signed-off-by: Kyle Xu zhenbing...@corigine.com
>
> Signed-off-by: Wentao Jia wentao@corigine.com
>
> Reviewed-by: Xinying Yu xinying...@corigine.com
>
> Reviewed-by: Shujing Dong shujing.d...@corigine.com
>
> Reviewed-by: Rick Zhong zhaoyong.zh...@corigine.com
>
> ---
>
> hw/net/vhost_net.c | 2 ++
>
> include/hw/virtio/virtio.h | 4
>
> 2 files changed, 6 insertions(+)
>
>
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>
> index e8e1661646..211ca859a6 100644
>
> --- a/hw/net/vhost_net.c
>
> +++ b/hw/net/vhost_net.c
>
> @@ -76,6 +76,8 @@ static const int user_feature_bits[] = {
>
> VIRTIO_F_IOMMU_PLATFORM,
>
> VIRTIO_F_RING_PACKED,
>
> VIRTIO_F_RING_RESET,
>
> +VIRTIO_F_IN_ORDER,
>
> +VIRTIO_F_NOTIFICATION_DATA,
>
> VIRTIO_NET_F_RSS,
>
> VIRTIO_NET_F_HASH_REPORT,
>
> VIRTIO_NET_F_GUEST_USO4,
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>
> index c8f72850bc..3880b6764c 100644
>
> --- a/include/hw/virtio/virtio.h
>
> +++ b/include/hw/virtio/virtio.h
>
> @@ -369,6 +369,10 @@ typedef struct VirtIORNGConf VirtIORNGConf;
>
>VIRTIO_F_RING_PACKED, false), \
>
> DEFINE_PROP_BIT64("queue_reset", _state, _field, \
>
>VIRTIO_F_RING_RESET, true)
>
> +DEFINE_PROP_BIT64("notification_data", _state, _field, \
>
> + VIRTIO_F_NOTIFICATION_DATA, true), \
>
> +DEFINE_PROP_BIT64("in_order", _state, _field, \
>
> + VIRTIO_F_IN_ORDER, true)
Do we want compatibility support for those?
Thanks
>
>
>
> hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
>
> bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
>
> --
Re: [PATCH 04/12] tests/plugin/inline: migrate to new per_vcpu API
On 1/12/24 14:51, Pierrick Bouvier wrote:
On 1/12/24 02:10, Richard Henderson wrote:
On 1/12/24 01:23, Pierrick Bouvier wrote:
Signed-off-by: Pierrick Bouvier
---
tests/plugin/inline.c | 17 -
1 file changed, 17 deletions(-)
Was this supposed to be together with patch 6?
My goal was to have a version that still uses original API.
If you prefer this to be squashed, no problem to do it.
My confusion is that this patch does not "migrate" anything -- it only removes code. Is
the just that the description is inaccurate? But it appears that the combination of 4+6
would "migrate" to the new API.
r~
r~
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 6114ebca545..ae59f7af7a7 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -18,15 +18,12 @@
static uint64_t count_tb;
static uint64_t count_tb_per_vcpu[MAX_CPUS];
static uint64_t count_tb_inline_per_vcpu[MAX_CPUS];
-static uint64_t count_tb_inline_racy;
static uint64_t count_insn;
static uint64_t count_insn_per_vcpu[MAX_CPUS];
static uint64_t count_insn_inline_per_vcpu[MAX_CPUS];
-static uint64_t count_insn_inline_racy;
static uint64_t count_mem;
static uint64_t count_mem_per_vcpu[MAX_CPUS];
static uint64_t count_mem_inline_per_vcpu[MAX_CPUS];
-static uint64_t count_mem_inline_racy;
static GMutex tb_lock;
static GMutex insn_lock;
static GMutex mem_lock;
@@ -50,11 +47,9 @@ static void stats_insn(void)
printf("insn: %" PRIu64 "\n", expected);
printf("insn: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("insn: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
- printf("insn: %" PRIu64 " (inline racy)\n", count_insn_inline_racy);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
- g_assert(count_insn_inline_racy <= expected);
}
static void stats_tb(void)
@@ -65,11 +60,9 @@ static void stats_tb(void)
printf("tb: %" PRIu64 "\n", expected);
printf("tb: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("tb: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
- printf("tb: %" PRIu64 " (inline racy)\n", count_tb_inline_racy);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
- g_assert(count_tb_inline_racy <= expected);
}
static void stats_mem(void)
@@ -80,11 +73,9 @@ static void stats_mem(void)
printf("mem: %" PRIu64 "\n", expected);
printf("mem: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("mem: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
- printf("mem: %" PRIu64 " (inline racy)\n", count_mem_inline_racy);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
- g_assert(count_mem_inline_racy <= expected);
}
static void plugin_exit(qemu_plugin_id_t id, void *udata)
@@ -142,8 +133,6 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
{
qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
QEMU_PLUGIN_CB_NO_REGS, 0);
- qemu_plugin_register_vcpu_tb_exec_inline(tb, QEMU_PLUGIN_INLINE_ADD_U64,
- &count_tb_inline_racy, 1);
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
tb, QEMU_PLUGIN_INLINE_ADD_U64,
count_tb_inline_per_vcpu, sizeof(uint64_t), 1);
@@ -152,18 +141,12 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx);
qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
QEMU_PLUGIN_CB_NO_REGS, 0);
- qemu_plugin_register_vcpu_insn_exec_inline(
- insn, QEMU_PLUGIN_INLINE_ADD_U64,
- &count_insn_inline_racy, 1);
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
insn, QEMU_PLUGIN_INLINE_ADD_U64,
count_insn_inline_per_vcpu, sizeof(uint64_t), 1);
qemu_plugin_register_vcpu_mem_cb(insn, &vcpu_mem_access,
QEMU_PLUGIN_CB_NO_REGS,
QEMU_PLUGIN_MEM_RW, 0);
- qemu_plugin_register_vcpu_mem_inline(insn, QEMU_PLUGIN_MEM_RW,
- QEMU_PLUGIN_INLINE_ADD_U64,
- &count_mem_inline_racy, 1);
qemu_plugin_register_vcpu_mem_inline_per_vcpu(
insn, QEMU_PLUGIN_MEM_RW,
QEMU_PLUGIN_INLINE_ADD_U64,
Re: [PATCH 00/33] hw/cpu/arm: Remove one use of qemu_get_cpu() in A7/A15 MPCore priv
On 1/10/24 07:03, Markus Armbruster wrote:
Peter Xu writes:
On Tue, Jan 09, 2024 at 10:22:31PM +0100, Philippe Mathieu-Daudé wrote:
Hi Fabiano,
On 9/1/24 21:21, Fabiano Rosas wrote:
Cédric Le Goater writes:
On 1/9/24 18:40, Fabiano Rosas wrote:
Cédric Le Goater writes:
On 1/3/24 20:53, Fabiano Rosas wrote:
Philippe Mathieu-Daudé writes:
+Peter/Fabiano
On 2/1/24 17:41, Cédric Le Goater wrote:
On 1/2/24 17:15, Philippe Mathieu-Daudé wrote:
Hi Cédric,
On 2/1/24 15:55, Cédric Le Goater wrote:
On 12/12/23 17:29, Philippe Mathieu-Daudé wrote:
Hi,
When a MPCore cluster is used, the Cortex-A cores belong the the
cluster container, not to the board/soc layer. This series move
the creation of vCPUs to the MPCore private container.
Doing so we consolidate the QOM model, moving common code in a
central place (abstract MPCore parent).
Changing the QOM hierarchy has an impact on the state of the machine
and some fixups are then required to maintain migration compatibility.
This can become a real headache for KVM machines like virt for which
migration compatibility is a feature, less for emulated ones.
All changes are either moving properties (which are not migrated)
or moving non-migrated QOM members (i.e. pointers of ARMCPU, which
is still migrated elsewhere). So I don't see any obvious migration
problem, but I might be missing something, so I Cc'ed Juan :>
FWIW, I didn't spot anything problematic either.
I've ran this through my migration compatibility series [1] and it
doesn't regress aarch64 migration from/to 8.2. The tests use '-M
virt -cpu max', so the cortex-a7 and cortex-a15 are not covered. I don't
think we even support migration of anything non-KVM on arm.
it happens we do.
Oh, sorry, I didn't mean TCG here. Probably meant to say something like
non-KVM-capable cpus, as in 32-bit. Nevermind.
Theoretically, we should be able to migrate to a TCG guest. Well, this
worked in the past for PPC. When I was doing more KVM related changes,
this was very useful for dev. Also, some machines are partially emulated.
Anyhow I agree this is not a strong requirement and we often break it.
Let's focus on KVM only.
1- https://gitlab.com/farosas/qemu/-/jobs/5853599533
yes it depends on the QOM hierarchy and virt seems immune to the changes.
Good.
However, changing the QOM topology clearly breaks migration compat,
Well, "clearly" is relative =) You've mentioned pseries and aspeed
already, do you have a pointer to one of those cases were we broke
migration
Regarding pseries, migration compat broke because of 5bc8d26de20c
("spapr: allocate the ICPState object from under sPAPRCPUCore") which
is similar to the changes proposed by this series, it impacts the QOM
hierarchy. Here is the workaround/fix from Greg : 46f7afa37096
("spapr: fix migration of ICPState objects from/to older QEMU") which
is quite an headache and this turned out to raise another problem some
months ago ... :/ That's why I sent [1] to prepare removal of old
machines and workarounds becoming a burden.
This feels like something that could be handled by the vmstate code
somehow. The state is there, just under a different path.
What, the QOM path is used in migration? ...
Hopefully not..
See recent discussions on "QOM path stability":
https://lore.kernel.org/qemu-devel/zzfyvlmcxbcia...@redhat.com/
https://lore.kernel.org/qemu-devel/87jzojbxt7@pond.sub.org/
https://lore.kernel.org/qemu-devel/87v883by34@pond.sub.org/
If I read it right, the commit 46f7afa37096 example is pretty special that
the QOM path more or less decided more than the hierachy itself but changes
the existances of objects.
Let's see whether I got this...
We removed some useless objects, moved the useful ones to another home.
The move changed their QOM path.
They interrupt controller presenter objects were quite useful :)
From what I recall, we moved them from an array under the machine
to the CPU object, so the interrupt controller presenter states
previously under the machine were not there anymore and this broke
migration compatibility.
Sorry for the noise if this is not a problem anymore. It was at
the time and we found a way to work around it; I should probably
say we hacked our way around it. Nevertheless, this was kind of
a trauma too because since I never dared touch the QOM hierarchy
of a migratable machine again. Migration is complicated.
The problem was the removal of useless objects, because this also
removed their vmstate.
The fix was adding the vmstate back as a dummy.
The QOM patch changes are *not* part of the problem.
Correct?
No one wants
to be policing QOM hierarchy changes in every single series that shows
up on the list.
Anyway, thanks for the pointers. I'll study that code a bit more, maybe
I can come up with some way to handle these cases.
Hopefully between the analyze-migration test and the compat tests we'll
catch the next bug of this kind before it gets merged.
Things like that might be able t
Re: [PATCH 09/12] contrib/plugins/hotblocks: migrate to new per_vcpu API
On 1/12/24 01:23, Pierrick Bouvier wrote: Signed-off-by: Pierrick Bouvier --- contrib/plugins/hotblocks.c | 25 +++-- 1 file changed, 19 insertions(+), 6 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH 10/12] contrib/plugins/howvec: migrate to new per_vcpu API
On 1/12/24 01:23, Pierrick Bouvier wrote:
@@ -180,11 +190,11 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
class = &class_table[i];
switch (class->what) {
case COUNT_CLASS:
-if (class->count || verbose) {
+if (count(class->count) || verbose) {
g_string_append_printf(report,
"Class: %-24s\t(%" PRId64 " hits)\n",
class->class,
- class->count);
+ count(class->count));
}
Compute count once. Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH 04/12] tests/plugin/inline: migrate to new per_vcpu API
On 1/12/24 12:40, Richard Henderson wrote:
On 1/12/24 14:51, Pierrick Bouvier wrote:
On 1/12/24 02:10, Richard Henderson wrote:
On 1/12/24 01:23, Pierrick Bouvier wrote:
Signed-off-by: Pierrick Bouvier
---
tests/plugin/inline.c | 17 -
1 file changed, 17 deletions(-)
Was this supposed to be together with patch 6?
My goal was to have a version that still uses original API.
If you prefer this to be squashed, no problem to do it.
My confusion is that this patch does not "migrate" anything -- it only removes
code. Is
the just that the description is inaccurate? But it appears that the
combination of 4+6
would "migrate" to the new API.
You're right, the commit message is incorrect, as it is just removing
the use of old API. Well, I think having this in a split commit does not
create any value for this serie, so I'll simply squash this in previous one.
r~
r~
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index 6114ebca545..ae59f7af7a7 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -18,15 +18,12 @@
static uint64_t count_tb;
static uint64_t count_tb_per_vcpu[MAX_CPUS];
static uint64_t count_tb_inline_per_vcpu[MAX_CPUS];
-static uint64_t count_tb_inline_racy;
static uint64_t count_insn;
static uint64_t count_insn_per_vcpu[MAX_CPUS];
static uint64_t count_insn_inline_per_vcpu[MAX_CPUS];
-static uint64_t count_insn_inline_racy;
static uint64_t count_mem;
static uint64_t count_mem_per_vcpu[MAX_CPUS];
static uint64_t count_mem_inline_per_vcpu[MAX_CPUS];
-static uint64_t count_mem_inline_racy;
static GMutex tb_lock;
static GMutex insn_lock;
static GMutex mem_lock;
@@ -50,11 +47,9 @@ static void stats_insn(void)
printf("insn: %" PRIu64 "\n", expected);
printf("insn: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("insn: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
- printf("insn: %" PRIu64 " (inline racy)\n", count_insn_inline_racy);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
- g_assert(count_insn_inline_racy <= expected);
}
static void stats_tb(void)
@@ -65,11 +60,9 @@ static void stats_tb(void)
printf("tb: %" PRIu64 "\n", expected);
printf("tb: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("tb: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
- printf("tb: %" PRIu64 " (inline racy)\n", count_tb_inline_racy);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
- g_assert(count_tb_inline_racy <= expected);
}
static void stats_mem(void)
@@ -80,11 +73,9 @@ static void stats_mem(void)
printf("mem: %" PRIu64 "\n", expected);
printf("mem: %" PRIu64 " (per vcpu)\n", per_vcpu);
printf("mem: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
- printf("mem: %" PRIu64 " (inline racy)\n", count_mem_inline_racy);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
- g_assert(count_mem_inline_racy <= expected);
}
static void plugin_exit(qemu_plugin_id_t id, void *udata)
@@ -142,8 +133,6 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
{
qemu_plugin_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
QEMU_PLUGIN_CB_NO_REGS, 0);
- qemu_plugin_register_vcpu_tb_exec_inline(tb, QEMU_PLUGIN_INLINE_ADD_U64,
- &count_tb_inline_racy, 1);
qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
tb, QEMU_PLUGIN_INLINE_ADD_U64,
count_tb_inline_per_vcpu, sizeof(uint64_t), 1);
@@ -152,18 +141,12 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, idx);
qemu_plugin_register_vcpu_insn_exec_cb(insn, vcpu_insn_exec,
QEMU_PLUGIN_CB_NO_REGS, 0);
- qemu_plugin_register_vcpu_insn_exec_inline(
- insn, QEMU_PLUGIN_INLINE_ADD_U64,
- &count_insn_inline_racy, 1);
qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
insn, QEMU_PLUGIN_INLINE_ADD_U64,
count_insn_inline_per_vcpu, sizeof(uint64_t), 1);
qemu_plugin_register_vcpu_mem_cb(insn, &vcpu_mem_access,
QEMU_PLUGIN_CB_NO_REGS,
QEMU_PLUGIN_MEM_RW, 0);
- qemu_plugin_register_vcpu_mem_inline(insn, QEMU_PLUGIN_MEM_RW,
- QEMU_PLUGIN_INLINE_ADD_U64,
- &count_mem_inline_racy, 1);
qemu_plugin_register_vcpu_mem_inline_per_vcpu(
insn, QEMU_PLUGIN_MEM_RW,
QEMU_PLUGIN_INL
Re: [PATCH 00/33] hw/cpu/arm: Remove one use of qemu_get_cpu() in A7/A15 MPCore priv
On 1/10/24 07:26, Peter Xu wrote:
On Wed, Jan 10, 2024 at 07:03:06AM +0100, Markus Armbruster wrote:
Peter Xu writes:
On Tue, Jan 09, 2024 at 10:22:31PM +0100, Philippe Mathieu-Daudé wrote:
Hi Fabiano,
On 9/1/24 21:21, Fabiano Rosas wrote:
Cédric Le Goater writes:
On 1/9/24 18:40, Fabiano Rosas wrote:
Cédric Le Goater writes:
On 1/3/24 20:53, Fabiano Rosas wrote:
Philippe Mathieu-Daudé writes:
+Peter/Fabiano
On 2/1/24 17:41, Cédric Le Goater wrote:
On 1/2/24 17:15, Philippe Mathieu-Daudé wrote:
Hi Cédric,
On 2/1/24 15:55, Cédric Le Goater wrote:
On 12/12/23 17:29, Philippe Mathieu-Daudé wrote:
Hi,
When a MPCore cluster is used, the Cortex-A cores belong the the
cluster container, not to the board/soc layer. This series move
the creation of vCPUs to the MPCore private container.
Doing so we consolidate the QOM model, moving common code in a
central place (abstract MPCore parent).
Changing the QOM hierarchy has an impact on the state of the machine
and some fixups are then required to maintain migration compatibility.
This can become a real headache for KVM machines like virt for which
migration compatibility is a feature, less for emulated ones.
All changes are either moving properties (which are not migrated)
or moving non-migrated QOM members (i.e. pointers of ARMCPU, which
is still migrated elsewhere). So I don't see any obvious migration
problem, but I might be missing something, so I Cc'ed Juan :>
FWIW, I didn't spot anything problematic either.
I've ran this through my migration compatibility series [1] and it
doesn't regress aarch64 migration from/to 8.2. The tests use '-M
virt -cpu max', so the cortex-a7 and cortex-a15 are not covered. I don't
think we even support migration of anything non-KVM on arm.
it happens we do.
Oh, sorry, I didn't mean TCG here. Probably meant to say something like
non-KVM-capable cpus, as in 32-bit. Nevermind.
Theoretically, we should be able to migrate to a TCG guest. Well, this
worked in the past for PPC. When I was doing more KVM related changes,
this was very useful for dev. Also, some machines are partially emulated.
Anyhow I agree this is not a strong requirement and we often break it.
Let's focus on KVM only.
1- https://gitlab.com/farosas/qemu/-/jobs/5853599533
yes it depends on the QOM hierarchy and virt seems immune to the changes.
Good.
However, changing the QOM topology clearly breaks migration compat,
Well, "clearly" is relative =) You've mentioned pseries and aspeed
already, do you have a pointer to one of those cases were we broke
migration
Regarding pseries, migration compat broke because of 5bc8d26de20c
("spapr: allocate the ICPState object from under sPAPRCPUCore") which
is similar to the changes proposed by this series, it impacts the QOM
hierarchy. Here is the workaround/fix from Greg : 46f7afa37096
("spapr: fix migration of ICPState objects from/to older QEMU") which
is quite an headache and this turned out to raise another problem some
months ago ... :/ That's why I sent [1] to prepare removal of old
machines and workarounds becoming a burden.
This feels like something that could be handled by the vmstate code
somehow. The state is there, just under a different path.
What, the QOM path is used in migration? ...
Hopefully not..
See recent discussions on "QOM path stability":
https://lore.kernel.org/qemu-devel/zzfyvlmcxbcia...@redhat.com/
https://lore.kernel.org/qemu-devel/87jzojbxt7@pond.sub.org/
https://lore.kernel.org/qemu-devel/87v883by34@pond.sub.org/
If I read it right, the commit 46f7afa37096 example is pretty special that
the QOM path more or less decided more than the hierachy itself but changes
the existances of objects.
Let's see whether I got this...
We removed some useless objects, moved the useful ones to another home.
The move changed their QOM path.
The problem was the removal of useless objects, because this also
removed their vmstate.
The fix was adding the vmstate back as a dummy.
The QOM patch changes are *not* part of the problem.
Correct?
[I'd leave this to Cedric]
No one wants
to be policing QOM hierarchy changes in every single series that shows
up on the list.
Anyway, thanks for the pointers. I'll study that code a bit more, maybe
I can come up with some way to handle these cases.
Hopefully between the analyze-migration test and the compat tests we'll
catch the next bug of this kind before it gets merged.
Things like that might be able to be detected via vmstate-static-checker.py.
But I'm not 100% sure, also its coverage is limited.
For example, I don't think it can detect changes to objects that will only
be created dynamically, e.g., I think sometimes we create objects after
some guest behaviors (consider guest enables the device, then QEMU
emulation creates some objects on demand of device setup?),
Feels nuts to me.
In real hardware, software enabling a device that is disabled by default
doesn't create the device. The device is always t
Re: [PULL 13/13] tests/avocado: remove skips from replay_kernel
Thomas Huth writes: > On 08/01/2024 16.13, Alex Bennée wrote: >> With the latest fixes for #2010 and #2013 these tests look pretty >> stable now. Of course the only way to be really sure is to run it in >> the CI infrastructure and see what breaks. >> Acked-by: Pavel Dovgalyuk >> Signed-off-by: Alex Bennée >> Message-Id: <20231211091346.14616-14-alex.ben...@linaro.org> > > The replay tests seem still to be very flaky, I'm now getting: > > https://gitlab.com/thuth/qemu/-/jobs/5910241580#L227 > https://gitlab.com/thuth/qemu/-/jobs/5910241593#L396 > > I'd suggest to revert this patch to disable them in the CI again. Ok I'll move it back to flaky. I thought I'd eliminated the problem as I can do 100 runs without failure locally. Sadly there doesn't seem to be anything obvious in the logs as to whats going on. > > Thomas -- Alex Bennée Virtualisation Tech Lead @ Linaro
[PATCH v5 0/3] Support RISC-V IOPMP
This series implements IOPMP specification v1.0.0-draft4 rapid-k model and add IOPMP device to RISC-V virt machine. Patch 1 add config STREAM make other device can reuse /hw/core/stream.c, IOPMP implementation will use it. Patch 2 implement IOPMP deivce. Patch 3 add IOPMP device to RISC-V virt machine. The IOPMP specification url: https://github.com/riscv-non-isa/iopmp-spec/blob/main/riscv_iopmp_specification.pdf Changes for v5: - Rebase - IOPMP: Support tracing (Alistair Francis) Use macros from registerfields.h for register (Alistair Francis) Support PCI device Drop IOPMP device create helper function (Alistair Francis) - Remove ATCDMAC300 (Alistair Francis) - VIRT: Make PCIe bridge connect to IOPMP Modify document for IOPMP options Add IOPMP fdt Thanks, Ethan Chen Ethan Chen (3): hw/core: Add config stream Add RISC-V IOPMP support hw/riscv/virt: Add IOPMP support docs/system/riscv/virt.rst| 12 + hw/Kconfig|1 + hw/core/Kconfig |3 + hw/core/meson.build |2 +- hw/misc/Kconfig |4 + hw/misc/meson.build |1 + hw/misc/riscv_iopmp.c | 1130 + hw/misc/trace-events |4 + hw/riscv/Kconfig |1 + hw/riscv/virt.c | 110 +- include/hw/misc/riscv_iopmp.h | 187 +++ .../hw/misc/riscv_iopmp_transaction_info.h| 28 + include/hw/riscv/virt.h |8 +- 13 files changed, 1488 insertions(+), 3 deletions(-) create mode 100644 hw/misc/riscv_iopmp.c create mode 100644 include/hw/misc/riscv_iopmp.h create mode 100644 include/hw/misc/riscv_iopmp_transaction_info.h -- 2.34.1
[PATCH v5 1/3] hw/core: Add config stream
Make other device can use /hw/core/stream.c by select this config.
Reviewed-by: Alistair Francis
Signed-off-by: Ethan Chen
---
hw/Kconfig | 1 +
hw/core/Kconfig | 3 +++
hw/core/meson.build | 2 +-
3 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/Kconfig b/hw/Kconfig
index 9ca7b38c31..e4d153dce7 100644
--- a/hw/Kconfig
+++ b/hw/Kconfig
@@ -79,6 +79,7 @@ config XILINX
config XILINX_AXI
bool
select PTIMER # for hw/dma/xilinx_axidma.c
+select STREAM
config XLNX_ZYNQMP
bool
diff --git a/hw/core/Kconfig b/hw/core/Kconfig
index 9397503656..e89ffa728b 100644
--- a/hw/core/Kconfig
+++ b/hw/core/Kconfig
@@ -27,3 +27,6 @@ config REGISTER
config SPLIT_IRQ
bool
+
+config STREAM
+bool
diff --git a/hw/core/meson.build b/hw/core/meson.build
index 67dad04de5..0893917b12 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -32,8 +32,8 @@ system_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true:
files('platform-bus.c'))
system_ss.add(when: 'CONFIG_PTIMER', if_true: files('ptimer.c'))
system_ss.add(when: 'CONFIG_REGISTER', if_true: files('register.c'))
system_ss.add(when: 'CONFIG_SPLIT_IRQ', if_true: files('split-irq.c'))
-system_ss.add(when: 'CONFIG_XILINX_AXI', if_true: files('stream.c'))
system_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('sysbus-fdt.c'))
+system_ss.add(when: 'CONFIG_STREAM', if_true: files('stream.c'))
system_ss.add(files(
'cpu-sysemu.c',
--
2.34.1
Re: [PATCH v4 1/4] linux-headers: drop pvpanic.h
On Sun, Jan 07 2024, Thomas Weißschuh wrote: > misc/pvpanic.h from the Linux UAPI does not define a Linux UAPI but a > qemu device API. > > This leads to a weird process when updates to the interface are needed: > 1) Change to the specification in the qemu tree > 2) Change to the header in the Linux tree > 3) Re-import of the header into Qemu. > > The kernel prefers to drop the header anyways. > > Prepare for the removal from the Linux UAPI headers by moving the > contents to the existing pvpanic.h header. > > Link: https://lore.kernel.org/lkml/2023110431-pacemaker-pruning-0e4c@gregkh/ > Signed-off-by: Thomas Weißschuh > --- > hw/misc/pvpanic-isa.c| 1 - > hw/misc/pvpanic-pci.c| 1 - > hw/misc/pvpanic.c| 1 - > include/hw/misc/pvpanic.h| 3 +++ > include/standard-headers/linux/pvpanic.h | 9 - > scripts/update-linux-headers.sh | 3 +-- > 6 files changed, 4 insertions(+), 14 deletions(-) Reviewed-by: Cornelia Huck
[PATCH v5 2/3] Add RISC-V IOPMP support
Support specification Version 1.0.0-draft4 rapid-k model.
The specification url:
https://github.com/riscv-non-isa/iopmp-spec/blob/main/riscv_iopmp_specification.pdf
The memory transaction from source devices connected to IOPMP will be
checked by IOPMP rule. The method of connecting the source device to
IOPMP as follows:
For a system bus device, device connects to IOPMP by calling
address_space_rw to address space iopmp_sysbus_as with
MemTxAttrs.requester_id=SID.
For a PCI bus device, device connects to IOPMP when it is on a host
bridge with iopmp_setup_pci. PCI bus device has default SID from PCI
BDF.
IOPMP have two optional features need source device support.
1. Partially hit detection: A Source device support IOPMP partially
hit detection need to send a transaction_info before transaction
start and send a transaction_info with eop after transaction end.
IOPMP will additionally check partially hit by transaction_info.
2. Stall: A Source device support IOPMP stall need to resend the
transaction when it gets the MEMTX_IOPMP_STALL result
There are three possible results of a transaction: valid, blocked, and
stalled. If a transaction is valid, target address space is
downstream_as(system_memory). If a transaction is blocked, it will go
to blocked_io_as. The operation of blocked_io_as could be a bus error,
a decode error, or it can respond a success with fabricated data
depending on IOPMP ERRREACT register value. If a transaction is
stalled, it will go to stall_io_as. The operation of stall_io_as does
nothing but return a stall result to source device. Source device
should retry the transaction if it gets a stall result.
Signed-off-by: Ethan Chen
---
hw/misc/Kconfig |4 +
hw/misc/meson.build |1 +
hw/misc/riscv_iopmp.c | 1130 +
hw/misc/trace-events |4 +
include/hw/misc/riscv_iopmp.h | 187 +++
.../hw/misc/riscv_iopmp_transaction_info.h| 28 +
6 files changed, 1354 insertions(+)
create mode 100644 hw/misc/riscv_iopmp.c
create mode 100644 include/hw/misc/riscv_iopmp.h
create mode 100644 include/hw/misc/riscv_iopmp_transaction_info.h
diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
index cc8a8c1418..953569e682 100644
--- a/hw/misc/Kconfig
+++ b/hw/misc/Kconfig
@@ -200,4 +200,8 @@ config IOSB
config XLNX_VERSAL_TRNG
bool
+config RISCV_IOPMP
+bool
+select STREAM
+
source macio/Kconfig
diff --git a/hw/misc/meson.build b/hw/misc/meson.build
index 36c20d5637..86b81e1690 100644
--- a/hw/misc/meson.build
+++ b/hw/misc/meson.build
@@ -35,6 +35,7 @@ system_ss.add(when: 'CONFIG_SIFIVE_E_PRCI', if_true:
files('sifive_e_prci.c'))
system_ss.add(when: 'CONFIG_SIFIVE_E_AON', if_true: files('sifive_e_aon.c'))
system_ss.add(when: 'CONFIG_SIFIVE_U_OTP', if_true: files('sifive_u_otp.c'))
system_ss.add(when: 'CONFIG_SIFIVE_U_PRCI', if_true: files('sifive_u_prci.c'))
+specific_ss.add(when: 'CONFIG_RISCV_IOPMP', if_true: files('riscv_iopmp.c'))
subdir('macio')
diff --git a/hw/misc/riscv_iopmp.c b/hw/misc/riscv_iopmp.c
new file mode 100644
index 00..468863bccf
--- /dev/null
+++ b/hw/misc/riscv_iopmp.c
@@ -0,0 +1,1130 @@
+/*
+ * QEMU RISC-V IOPMP (Input Output Physical Memory Protection)
+ *
+ * Copyright (c) 2023 Andes Tech. Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see
[PATCH v5 3/3] hw/riscv/virt: Add IOPMP support
If a source device is connected to the IOPMP device, its memory
transaction will be checked by the IOPMP rule.
When using RISC-V virt machine option "iopmp=on", the generic PCIe host
bridge connects to IOPMP. The PCI devices on the brigde will connets to
IOPMP with default source id(SID) from PCI BDF.
- Add 'iopmp=on' option to add an iopmp device. It checks dma
operations from the generic PCIe host bridge. This option is assumed
to be "off"
- Add 'iopmp_cascade=on' option to add second iopmp device which is
cascaded by first iopmp device to machine. When iopmp option is "off"
, this option has no effect.
Signed-off-by: Ethan Chen
---
docs/system/riscv/virt.rst | 12
hw/riscv/Kconfig | 1 +
hw/riscv/virt.c| 110 -
include/hw/riscv/virt.h| 8 ++-
4 files changed, 129 insertions(+), 2 deletions(-)
diff --git a/docs/system/riscv/virt.rst b/docs/system/riscv/virt.rst
index 9a06f95a34..e07ce2cd28 100644
--- a/docs/system/riscv/virt.rst
+++ b/docs/system/riscv/virt.rst
@@ -116,6 +116,18 @@ The following machine-specific options are supported:
having AIA IMSIC (i.e. "aia=aplic-imsic" selected). When not specified,
the default number of per-HART VS-level AIA IMSIC pages is 0.
+- iopmp=[on|off]
+
+ When this option is "on". An IOPMP device is added to machine. It checks dma
+ operations from the generic PCIe host bridge. This option is assumed to be
+ "off".
+
+- iopmp_cascade=[on|off]
+
+ When this option is "on". Second IOPMP device which is cascaded by first
IOPMP
+ device is added to machine. When IOPMP option is "off", this option has no
+ effect. This option is assumed to be "off".
+
Running Linux kernel
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index a50717be87..c207b94747 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -46,6 +46,7 @@ config RISCV_VIRT
select PLATFORM_BUS
select ACPI
select ACPI_PCI
+select RISCV_IOPMP
config SHAKTI_C
bool
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index f9fd1341fc..c7a5035074 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -52,6 +52,7 @@
#include "hw/display/ramfb.h"
#include "hw/acpi/aml-build.h"
#include "qapi/qapi-visit-common.h"
+#include "hw/misc/riscv_iopmp.h"
/* KVM AIA only supports APLIC MSI. APLIC Wired is always emulated by QEMU. */
static bool virt_use_kvm_aia(RISCVVirtState *s)
@@ -74,6 +75,8 @@ static const MemMapEntry virt_memmap[] = {
[VIRT_UART0] ={ 0x1000, 0x100 },
[VIRT_VIRTIO] = { 0x10001000,0x1000 },
[VIRT_FW_CFG] = { 0x1010, 0x18 },
+[VIRT_IOPMP] ={ 0x1020, 0x10 },
+[VIRT_IOPMP2] = { 0x1030, 0x10 },
[VIRT_FLASH] ={ 0x2000, 0x400 },
[VIRT_IMSIC_M] = { 0x2400, VIRT_IMSIC_MAX_SIZE },
[VIRT_IMSIC_S] = { 0x2800, VIRT_IMSIC_MAX_SIZE },
@@ -1011,6 +1014,44 @@ static void create_fdt_fw_cfg(RISCVVirtState *s, const
MemMapEntry *memmap)
g_free(nodename);
}
+static void create_fdt_iopmp(RISCVVirtState *s, const MemMapEntry *memmap,
+ uint32_t irq_mmio_phandle) {
+char *name;
+MachineState *ms = MACHINE(s);
+
+name = g_strdup_printf("/soc/iopmp@%lx", (long)memmap[VIRT_IOPMP].base);
+qemu_fdt_add_subnode(ms->fdt, name);
+qemu_fdt_setprop_string(ms->fdt, name, "compatible", "riscv_iopmp");
+qemu_fdt_setprop_cells(ms->fdt, name, "reg", 0x0, memmap[VIRT_IOPMP].base,
+0x0, memmap[VIRT_IOPMP].size);
+qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent", irq_mmio_phandle);
+if (s->aia_type == VIRT_AIA_TYPE_NONE) {
+qemu_fdt_setprop_cell(ms->fdt, name, "interrupts", IOPMP_IRQ);
+} else {
+qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", IOPMP_IRQ, 0x4);
+}
+g_free(name);
+}
+
+static void create_fdt_iopmp2(RISCVVirtState *s, const MemMapEntry *memmap,
+ uint32_t irq_mmio_phandle) {
+char *name;
+MachineState *ms = MACHINE(s);
+
+name = g_strdup_printf("/soc/iopmp2@%lx", (long)memmap[VIRT_IOPMP2].base);
+qemu_fdt_add_subnode(ms->fdt, name);
+qemu_fdt_setprop_string(ms->fdt, name, "compatible", "riscv_iopmp");
+qemu_fdt_setprop_cells(ms->fdt, name, "reg", 0x0, memmap[VIRT_IOPMP2].base,
+0x0, memmap[VIRT_IOPMP2].size);
+qemu_fdt_setprop_cell(ms->fdt, name, "interrupt-parent", irq_mmio_phandle);
+if (s->aia_type == VIRT_AIA_TYPE_NONE) {
+qemu_fdt_setprop_cell(ms->fdt, name, "interrupts", IOPMP2_IRQ);
+} else {
+qemu_fdt_setprop_cells(ms->fdt, name, "interrupts", IOPMP2_IRQ, 0x4);
+}
+g_free(name);
+}
+
static void finalize_fdt(RISCVVirtState *s)
{
uint32_t phandle = 1, irq_mmio_phandle = 1, msi_pcie_phandle = 1;
@@ -1029,6 +1070,13 @@ static void finalize_fdt(RISCVVirtState *s)
create_fdt_uart(s, virt_memmap, irq
Re: [PATCH v4 2/4] hw/misc/pvpanic: centralize definition of supported events
On Sun, Jan 07 2024, Thomas Weißschuh wrote: > The different components of pvpanic duplicate the list of supported > events. Move it to the shared header file to minimize changes when new > events are added. > > Signed-off-by: Thomas Weißschuh > --- > hw/misc/pvpanic-isa.c | 2 +- > hw/misc/pvpanic-pci.c | 2 +- > hw/misc/pvpanic.c | 2 +- > include/hw/misc/pvpanic.h | 1 + > 4 files changed, 4 insertions(+), 3 deletions(-) Reviewed-by: Cornelia Huck
Re: [PATCH v4 3/4] tests/qtest/pvpanic: use centralized definition of supported events
On Sun, Jan 07 2024, Thomas Weißschuh wrote: > Avoid the necessity to update all tests when new events are added > to the device. > > Acked-by: Thomas Huth > Signed-off-by: Thomas Weißschuh > --- > tests/qtest/pvpanic-pci-test.c | 5 +++-- > tests/qtest/pvpanic-test.c | 5 +++-- > 2 files changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Cornelia Huck
Re: [PATCH v4 4/4] hw/misc/pvpanic: add support for normal shutdowns
On Sun, Jan 07 2024, Thomas Weißschuh wrote: > Shutdown requests are normally hardware dependent. > By extending pvpanic to also handle shutdown requests, guests can > submit such requests with an easily implementable and cross-platform > mechanism. > > Signed-off-by: Thomas Weißschuh > --- > docs/specs/pvpanic.rst| 2 ++ > hw/misc/pvpanic.c | 5 + > include/hw/misc/pvpanic.h | 3 ++- > 3 files changed, 9 insertions(+), 1 deletion(-) Not deeply involved with this, but looks reasonable to me. Acked-by: Cornelia Huck
Re: [PATCH 1/2] gitlab: Introduce Loongarch64 runner
在 2024/1/11 下午4:26, Thomas Huth 写道:
On 11/01/2024 08.25, gaosong wrote:
Hi,
在 2024/1/11 下午3:08, Thomas Huth 写道:
On 02/01/2024 18.22, Philippe Mathieu-Daudé wrote:
Full build config to run CI tests on a Loongarch64 host.
Forks might enable this by setting LOONGARCH64_RUNNER_AVAILABLE
in their CI namespace settings, see:
https://www.qemu.org/docs/master/devel/ci.html#maintainer-controlled-job-variables
Signed-off-by: Philippe Mathieu-Daudé
---
docs/devel/ci-jobs.rst.inc | 6 ++
.gitlab-ci.d/custom-runners.yml | 1 +
.../openeuler-22.03-loongarch64.yml | 21
+++
3 files changed, 28 insertions(+)
create mode 100644
.gitlab-ci.d/custom-runners/openeuler-22.03-loongarch64.yml
...
diff --git a/.gitlab-ci.d/custom-runners.yml
b/.gitlab-ci.d/custom-runners.yml
index 8e5b9500f4..152ace4492 100644
--- a/.gitlab-ci.d/custom-runners.yml
+++ b/.gitlab-ci.d/custom-runners.yml
@@ -32,3 +32,4 @@ include:
- local: '/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml'
- local: '/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch32.yml'
- local: '/.gitlab-ci.d/custom-runners/centos-stream-8-x86_64.yml'
+ - local:
'/.gitlab-ci.d/custom-runners/openeuler-22.03-loongarch64.yml'
diff --git
a/.gitlab-ci.d/custom-runners/openeuler-22.03-loongarch64.yml
b/.gitlab-ci.d/custom-runners/openeuler-22.03-loongarch64.yml
new file mode 100644
index 00..86d18f820e
--- /dev/null
+++ b/.gitlab-ci.d/custom-runners/openeuler-22.03-loongarch64.yml
@@ -0,0 +1,21 @@
+openeuler-22.03-loongarch64-all:
+ extends: .custom_runner_template :-)
+ needs: []
+ stage: build
+ tags:
+ - oe2203
+ - loongarch64
+ rules:
+ - if: '$CI_PROJECT_NAMESPACE == "qemu-project" &&
$CI_COMMIT_BRANCH =~ /^staging/'
+ when: manual
+ allow_failure: true
+ - if: "$LOONGARCH64_RUNNER_AVAILABLE"
+ when: manual
+ allow_failure: true
+ script:
+ - mkdir build
+ - cd build
+ - ../configure
+ || { cat config.log meson-logs/meson-log.txt; exit 1; }
+ - make --output-sync -j`nproc --ignore=40`
+ - make --output-sync -j`nproc --ignore=40` check
Does this system really have more than 40 CPU threads? Or is this a
copy-n-past from one of the other scripts? In the latter case, I'd
suggest to adjust the --ignore=40 to a more reasonable value.
Thomas
No, only 32. I think it should be --ignore=32 or 16.
--ignore=32 then also does not make much sense, that would still be
the same as simply omitting the -j parameter. I guess --ignore=16
should be fine.
I create a same runner on this machine, and I find some check error.
but I am not sure how to fix it. :-)
See:
https://gitlab.com/gaosong/qemu/-/jobs/5906269934
Seems to be related to RAM backing... for example, the erst-test is
failing, which is doing something like:
setup_vm_cmd(&state,
"-object memory-backend-file,"
"mem-path=acpi-erst.XX,"
"size=64K,"
Hi,
We tested this on
LoongArch host with the 5.10 kernel, (openEuler 22.03),
x86_64 host with 5.10 kernel, (openEuler 22.03)
x86_64 host with 5.15kernel , (Ubuntu 20.04.3 LTS)
and didn't get any error.
but the CI machine use the 6.7_rc4 kernel.
we didn't update the x86_64 host kernel to tested this.
Is it possible that the new kernel is causing the problem?
"share=on,"
"id=nvram "
"-device acpi-erst,"
"memdev=nvram");
So it seems like -object memory-backend-file" is not correctly working
in your gitlab runner? Is there some setup missing?
Thomas
[PATCH 2/5] qemu-options: Remove the deprecated -no-acpi option
It's been marked as deprecated since QEMU 8.0, so it should be fine
to remove this now.
Signed-off-by: Thomas Huth
---
docs/about/deprecated.rst | 6 --
docs/about/removed-features.rst | 5 +
system/vl.c | 4
qemu-options.hx | 9 -
4 files changed, 5 insertions(+), 19 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 6c668b8531..dff4c76f1b 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -63,12 +63,6 @@ as short-form boolean values, and passed to plugins as
``arg_name=on``.
However, short-form booleans are deprecated and full explicit ``arg_name=on``
form is preferred.
-``-no-acpi`` (since 8.0)
-
-
-The ``-no-acpi`` setting has been turned into a machine property.
-Use ``-machine acpi=off`` instead.
-
``-async-teardown`` (since 8.1)
'''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 52d240ade9..ae728b6130 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -466,6 +466,11 @@ line if the ``-nodefaults`` options is used.
The HPET setting has been turned into a machine property.
Use ``-machine hpet=off`` instead.
+``-no-acpi`` (removed in 9.0)
+'
+
+The ``-no-acpi`` setting has been turned into a machine property.
+Use ``-machine acpi=off`` instead.
QEMU Machine Protocol (QMP) commands
diff --git a/system/vl.c b/system/vl.c
index f08c4c8193..7e258889f3 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3369,10 +3369,6 @@ void qemu_init(int argc, char **argv)
display_remote++;
break;
#endif
-case QEMU_OPTION_no_acpi:
-warn_report("-no-acpi is deprecated, use '-machine acpi=off'
instead");
-qdict_put_str(machine_opts_dict, "acpi", "off");
-break;
case QEMU_OPTION_no_reboot:
olist = qemu_find_opts("action");
qemu_opts_parse_noisily(olist, "reboot=shutdown", false);
diff --git a/qemu-options.hx b/qemu-options.hx
index 57e447065a..dafecf47d6 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2645,15 +2645,6 @@ SRST
needed to boot from old floppy disks.
ERST
-DEF("no-acpi", 0, QEMU_OPTION_no_acpi,
- "-no-acpidisable ACPI\n", QEMU_ARCH_I386 | QEMU_ARCH_ARM)
-SRST
-``-no-acpi``
-Disable ACPI (Advanced Configuration and Power Interface) support.
-Use it if your guest OS complains about ACPI problems (PC target
-machine only).
-ERST
-
DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
"-acpitable
[sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"
"ACPI table description\n", QEMU_ARCH_I386)
--
2.43.0
[PATCH 1/5] qemu-options: Remove the deprecated -no-hpet option
It's been marked as deprecated since QEMU 8.0, so it should be fine
to remove this now.
Signed-off-by: Thomas Huth
---
docs/about/deprecated.rst | 6 --
docs/about/removed-features.rst | 8
system/vl.c | 4
qemu-options.hx | 7 ---
4 files changed, 8 insertions(+), 17 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7fdd2239b4..6c668b8531 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -63,12 +63,6 @@ as short-form boolean values, and passed to plugins as
``arg_name=on``.
However, short-form booleans are deprecated and full explicit ``arg_name=on``
form is preferred.
-``-no-hpet`` (since 8.0)
-
-
-The HPET setting has been turned into a machine property.
-Use ``-machine hpet=off`` instead.
-
``-no-acpi`` (since 8.0)
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index f04036987b..52d240ade9 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -460,6 +460,14 @@ in this case.
Note that the default audio backend must be configured on the command
line if the ``-nodefaults`` options is used.
+``-no-hpet`` (removed in 9.0)
+'
+
+The HPET setting has been turned into a machine property.
+Use ``-machine hpet=off`` instead.
+
+
+
QEMU Machine Protocol (QMP) commands
diff --git a/system/vl.c b/system/vl.c
index 53850a1daf..f08c4c8193 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3373,10 +3373,6 @@ void qemu_init(int argc, char **argv)
warn_report("-no-acpi is deprecated, use '-machine acpi=off'
instead");
qdict_put_str(machine_opts_dict, "acpi", "off");
break;
-case QEMU_OPTION_no_hpet:
-warn_report("-no-hpet is deprecated, use '-machine hpet=off'
instead");
-qdict_put_str(machine_opts_dict, "hpet", "off");
-break;
case QEMU_OPTION_no_reboot:
olist = qemu_find_opts("action");
qemu_opts_parse_noisily(olist, "reboot=shutdown", false);
diff --git a/qemu-options.hx b/qemu-options.hx
index b66570ae00..57e447065a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2654,13 +2654,6 @@ SRST
machine only).
ERST
-DEF("no-hpet", 0, QEMU_OPTION_no_hpet,
-"-no-hpetdisable HPET\n", QEMU_ARCH_I386)
-SRST
-``-no-hpet``
-Disable HPET support. Deprecated, use '-machine hpet=off' instead.
-ERST
-
DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
"-acpitable
[sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"
"ACPI table description\n", QEMU_ARCH_I386)
--
2.43.0
[PATCH 3/5] qemu-options: Remove the deprecated -async-teardown option
It's been marked as deprecated since QEMU 8.1 (and was only available
since QEMU 8.0 anyway), so it should be fine to remove this now.
Signed-off-by: Thomas Huth
---
docs/about/deprecated.rst | 5 -
docs/about/removed-features.rst | 5 +
system/vl.c | 6 --
qemu-options.hx | 10 --
4 files changed, 5 insertions(+), 21 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index dff4c76f1b..80eacd40ba 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -63,11 +63,6 @@ as short-form boolean values, and passed to plugins as
``arg_name=on``.
However, short-form booleans are deprecated and full explicit ``arg_name=on``
form is preferred.
-``-async-teardown`` (since 8.1)
-'''
-
-Use ``-run-with async-teardown=on`` instead.
-
``-chroot`` (since 8.1)
'''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index ae728b6130..43f64a26ba 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -472,6 +472,11 @@ Use ``-machine hpet=off`` instead.
The ``-no-acpi`` setting has been turned into a machine property.
Use ``-machine acpi=off`` instead.
+``-async-teardown`` (removed in 9.0)
+
+
+Use ``-run-with async-teardown=on`` instead.
+
QEMU Machine Protocol (QMP) commands
diff --git a/system/vl.c b/system/vl.c
index 7e258889f3..924356f864 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3600,12 +3600,6 @@ void qemu_init(int argc, char **argv)
case QEMU_OPTION_daemonize:
os_set_daemonize(true);
break;
-#if defined(CONFIG_LINUX)
-/* deprecated */
-case QEMU_OPTION_asyncteardown:
-init_async_teardown();
-break;
-#endif
case QEMU_OPTION_run_with: {
const char *str;
opts = qemu_opts_parse_noisily(qemu_find_opts("run-with"),
diff --git a/qemu-options.hx b/qemu-options.hx
index dafecf47d6..10c952ba3f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4975,16 +4975,6 @@ HXCOMM Internal use
DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL)
DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL)
-#ifdef __linux__
-DEF("async-teardown", 0, QEMU_OPTION_asyncteardown,
-"-async-teardown enable asynchronous teardown\n",
-QEMU_ARCH_ALL)
-SRST
-``-async-teardown``
-This option is deprecated and should no longer be used. The new option
-``-run-with async-teardown=on`` is a replacement.
-ERST
-#endif
#ifdef CONFIG_POSIX
DEF("run-with", HAS_ARG, QEMU_OPTION_run_with,
"-run-with [async-teardown=on|off][,chroot=dir]\n"
--
2.43.0
[PATCH 4/5] qemu-options: Remove the deprecated -chroot option
It's been marked as deprecated since QEMU 8.1, so it should be fine
to remove this now.
Signed-off-by: Thomas Huth
---
docs/about/deprecated.rst | 5 -
docs/about/removed-features.rst | 5 +
system/vl.c | 5 -
qemu-options.hx | 12
4 files changed, 5 insertions(+), 22 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 80eacd40ba..b50cfe596b 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -63,11 +63,6 @@ as short-form boolean values, and passed to plugins as
``arg_name=on``.
However, short-form booleans are deprecated and full explicit ``arg_name=on``
form is preferred.
-``-chroot`` (since 8.1)
-'''
-
-Use ``-run-with chroot=dir`` instead.
-
``-singlestep`` (since 8.1)
'''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 43f64a26ba..a8546f4787 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -477,6 +477,11 @@ Use ``-machine acpi=off`` instead.
Use ``-run-with async-teardown=on`` instead.
+``-chroot`` (removed in 9.0)
+
+
+Use ``-run-with chroot=dir`` instead.
+
QEMU Machine Protocol (QMP) commands
diff --git a/system/vl.c b/system/vl.c
index 924356f864..c125fb9079 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -3592,11 +3592,6 @@ void qemu_init(int argc, char **argv)
exit(1);
}
break;
-case QEMU_OPTION_chroot:
-warn_report("option is deprecated,"
-" use '-run-with chroot=...' instead");
-os_set_chroot(optarg);
-break;
case QEMU_OPTION_daemonize:
os_set_daemonize(true);
break;
diff --git a/qemu-options.hx b/qemu-options.hx
index 10c952ba3f..9be6beb5a0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4790,18 +4790,6 @@ SRST
``-nodefaults`` option will disable all those default devices.
ERST
-#ifndef _WIN32
-DEF("chroot", HAS_ARG, QEMU_OPTION_chroot, \
-"-chroot dir chroot to dir just before starting the VM (deprecated)\n",
-QEMU_ARCH_ALL)
-#endif
-SRST
-``-chroot dir``
-Deprecated, use '-run-with chroot=...' instead.
-Immediately before starting guest execution, chroot to the specified
-directory. Especially useful in combination with -runas.
-ERST
-
#ifndef _WIN32
DEF("runas", HAS_ARG, QEMU_OPTION_runas, \
"-runas user change to user id user just before starting the VM\n" \
--
2.43.0
[PATCH 0/5] Remove deprecated command line options
The -no-hpet, -no-acpi, -async-teardown, -chroot and -singlestep options have been deprecated for at least two releases, so it should be fine if we remove them now. Thomas Huth (5): qemu-options: Remove the deprecated -no-hpet option qemu-options: Remove the deprecated -no-acpi option qemu-options: Remove the deprecated -async-teardown option qemu-options: Remove the deprecated -chroot option qemu-options: Remove the deprecated -singlestep option docs/about/deprecated.rst | 28 docs/about/removed-features.rst | 30 + system/vl.c | 37 +- qemu-options.hx | 46 - 4 files changed, 31 insertions(+), 110 deletions(-) -- 2.43.0
[PATCH 5/5] qemu-options: Remove the deprecated -singlestep option
It's been marked as deprecated since QEMU 8.1, so it should be fine
to remove this now.
Signed-off-by: Thomas Huth
---
docs/about/deprecated.rst | 6 --
docs/about/removed-features.rst | 7 +++
system/vl.c | 18 +-
qemu-options.hx | 8
4 files changed, 8 insertions(+), 31 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index b50cfe596b..81a5149f1e 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -63,12 +63,6 @@ as short-form boolean values, and passed to plugins as
``arg_name=on``.
However, short-form booleans are deprecated and full explicit ``arg_name=on``
form is preferred.
-``-singlestep`` (since 8.1)
-'''
-
-The ``-singlestep`` option has been turned into an accelerator property,
-and given a name that better reflects what it actually does.
-Use ``-accel tcg,one-insn-per-tb=on`` instead.
User-mode emulator command line arguments
-
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index a8546f4787..d5c60ff47f 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -482,6 +482,13 @@ Use ``-run-with async-teardown=on`` instead.
Use ``-run-with chroot=dir`` instead.
+``-singlestep`` (removed in 9.0)
+
+
+The ``-singlestep`` option has been turned into an accelerator property,
+and given a name that better reflects what it actually does.
+Use ``-accel tcg,one-insn-per-tb=on`` instead.
+
QEMU Machine Protocol (QMP) commands
diff --git a/system/vl.c b/system/vl.c
index c125fb9079..809f867bcc 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -181,7 +181,6 @@ static const char *log_file;
static bool list_data_dirs;
static const char *qtest_chrdev;
static const char *qtest_log;
-static bool opt_one_insn_per_tb;
static int has_defaults = 1;
static int default_audio = 1;
@@ -2308,19 +2307,7 @@ static int do_configure_accelerator(void *opaque,
QemuOpts *opts, Error **errp)
qemu_opt_foreach(opts, accelerator_set_property,
accel,
&error_fatal);
-/*
- * If legacy -singlestep option is set, honour it for TCG and
- * silently ignore for any other accelerator (which is how this
- * option has always behaved).
- */
-if (opt_one_insn_per_tb) {
-/*
- * This will always succeed for TCG, and we want to ignore
- * the error from trying to set a nonexistent property
- * on any other accelerator.
- */
-object_property_set_bool(OBJECT(accel), "one-insn-per-tb", true, NULL);
-}
+
ret = accel_init_machine(accel, current_machine);
if (ret < 0) {
if (!qtest_with_kvm || ret != -ENOENT) {
@@ -3057,9 +3044,6 @@ void qemu_init(int argc, char **argv)
case QEMU_OPTION_bios:
qdict_put_str(machine_opts_dict, "firmware", optarg);
break;
-case QEMU_OPTION_singlestep:
-opt_one_insn_per_tb = true;
-break;
case QEMU_OPTION_S:
autostart = 0;
break;
diff --git a/qemu-options.hx b/qemu-options.hx
index 9be6beb5a0..033fa873e4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4357,14 +4357,6 @@ SRST
from a script.
ERST
-DEF("singlestep", 0, QEMU_OPTION_singlestep, \
-"-singlestep deprecated synonym for -accel tcg,one-insn-per-tb=on\n",
QEMU_ARCH_ALL)
-SRST
-``-singlestep``
-This is a deprecated synonym for the TCG accelerator property
-``one-insn-per-tb``.
-ERST
-
DEF("preconfig", 0, QEMU_OPTION_preconfig, \
"--preconfig pause QEMU before machine is initialized
(experimental)\n",
QEMU_ARCH_ALL)
--
2.43.0
Re: [PATCH 3/5] qemu-options: Remove the deprecated -async-teardown option
On Fri, 12 Jan 2024 11:00:57 +0100
Thomas Huth wrote:
> It's been marked as deprecated since QEMU 8.1 (and was only available
> since QEMU 8.0 anyway), so it should be fine to remove this now.
>
> Signed-off-by: Thomas Huth
Reviewed-by: Claudio Imbrenda
> ---
> docs/about/deprecated.rst | 5 -
> docs/about/removed-features.rst | 5 +
> system/vl.c | 6 --
> qemu-options.hx | 10 --
> 4 files changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index dff4c76f1b..80eacd40ba 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -63,11 +63,6 @@ as short-form boolean values, and passed to plugins as
> ``arg_name=on``.
> However, short-form booleans are deprecated and full explicit ``arg_name=on``
> form is preferred.
>
> -``-async-teardown`` (since 8.1)
> -'''
> -
> -Use ``-run-with async-teardown=on`` instead.
> -
> ``-chroot`` (since 8.1)
> '''
>
> diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
> index ae728b6130..43f64a26ba 100644
> --- a/docs/about/removed-features.rst
> +++ b/docs/about/removed-features.rst
> @@ -472,6 +472,11 @@ Use ``-machine hpet=off`` instead.
> The ``-no-acpi`` setting has been turned into a machine property.
> Use ``-machine acpi=off`` instead.
>
> +``-async-teardown`` (removed in 9.0)
> +
> +
> +Use ``-run-with async-teardown=on`` instead.
> +
>
> QEMU Machine Protocol (QMP) commands
>
> diff --git a/system/vl.c b/system/vl.c
> index 7e258889f3..924356f864 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -3600,12 +3600,6 @@ void qemu_init(int argc, char **argv)
> case QEMU_OPTION_daemonize:
> os_set_daemonize(true);
> break;
> -#if defined(CONFIG_LINUX)
> -/* deprecated */
> -case QEMU_OPTION_asyncteardown:
> -init_async_teardown();
> -break;
> -#endif
> case QEMU_OPTION_run_with: {
> const char *str;
> opts = qemu_opts_parse_noisily(qemu_find_opts("run-with"),
> diff --git a/qemu-options.hx b/qemu-options.hx
> index dafecf47d6..10c952ba3f 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4975,16 +4975,6 @@ HXCOMM Internal use
> DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL)
> DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL)
>
> -#ifdef __linux__
> -DEF("async-teardown", 0, QEMU_OPTION_asyncteardown,
> -"-async-teardown enable asynchronous teardown\n",
> -QEMU_ARCH_ALL)
> -SRST
> -``-async-teardown``
> -This option is deprecated and should no longer be used. The new option
> -``-run-with async-teardown=on`` is a replacement.
> -ERST
> -#endif
> #ifdef CONFIG_POSIX
> DEF("run-with", HAS_ARG, QEMU_OPTION_run_with,
> "-run-with [async-teardown=on|off][,chroot=dir]\n"
Re: [PATCH 00/33] hw/cpu/arm: Remove one use of qemu_get_cpu() in A7/A15 MPCore priv
On 1/10/24 14:19, Fabiano Rosas wrote:
Markus Armbruster writes:
Peter Xu writes:
On Tue, Jan 09, 2024 at 10:22:31PM +0100, Philippe Mathieu-Daudé wrote:
Hi Fabiano,
On 9/1/24 21:21, Fabiano Rosas wrote:
Cédric Le Goater writes:
On 1/9/24 18:40, Fabiano Rosas wrote:
Cédric Le Goater writes:
On 1/3/24 20:53, Fabiano Rosas wrote:
Philippe Mathieu-Daudé writes:
+Peter/Fabiano
On 2/1/24 17:41, Cédric Le Goater wrote:
On 1/2/24 17:15, Philippe Mathieu-Daudé wrote:
Hi Cédric,
On 2/1/24 15:55, Cédric Le Goater wrote:
On 12/12/23 17:29, Philippe Mathieu-Daudé wrote:
Hi,
When a MPCore cluster is used, the Cortex-A cores belong the the
cluster container, not to the board/soc layer. This series move
the creation of vCPUs to the MPCore private container.
Doing so we consolidate the QOM model, moving common code in a
central place (abstract MPCore parent).
Changing the QOM hierarchy has an impact on the state of the machine
and some fixups are then required to maintain migration compatibility.
This can become a real headache for KVM machines like virt for which
migration compatibility is a feature, less for emulated ones.
All changes are either moving properties (which are not migrated)
or moving non-migrated QOM members (i.e. pointers of ARMCPU, which
is still migrated elsewhere). So I don't see any obvious migration
problem, but I might be missing something, so I Cc'ed Juan :>
FWIW, I didn't spot anything problematic either.
I've ran this through my migration compatibility series [1] and it
doesn't regress aarch64 migration from/to 8.2. The tests use '-M
virt -cpu max', so the cortex-a7 and cortex-a15 are not covered. I don't
think we even support migration of anything non-KVM on arm.
it happens we do.
Oh, sorry, I didn't mean TCG here. Probably meant to say something like
non-KVM-capable cpus, as in 32-bit. Nevermind.
Theoretically, we should be able to migrate to a TCG guest. Well, this
worked in the past for PPC. When I was doing more KVM related changes,
this was very useful for dev. Also, some machines are partially emulated.
Anyhow I agree this is not a strong requirement and we often break it.
Let's focus on KVM only.
1- https://gitlab.com/farosas/qemu/-/jobs/5853599533
yes it depends on the QOM hierarchy and virt seems immune to the changes.
Good.
However, changing the QOM topology clearly breaks migration compat,
Well, "clearly" is relative =) You've mentioned pseries and aspeed
already, do you have a pointer to one of those cases were we broke
migration
Regarding pseries, migration compat broke because of 5bc8d26de20c
("spapr: allocate the ICPState object from under sPAPRCPUCore") which
is similar to the changes proposed by this series, it impacts the QOM
hierarchy. Here is the workaround/fix from Greg : 46f7afa37096
("spapr: fix migration of ICPState objects from/to older QEMU") which
is quite an headache and this turned out to raise another problem some
months ago ... :/ That's why I sent [1] to prepare removal of old
machines and workarounds becoming a burden.
This feels like something that could be handled by the vmstate code
somehow. The state is there, just under a different path.
What, the QOM path is used in migration? ...
Hopefully not..
Unfortunately the original fix doesn't mention _what_ actually broke
with migration. I assumed the QOM path was needed because otherwise I
don't think the fix makes sense. The thread discussing that patch also
directly mentions the QOM path:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg450912.html
But I probably misunderstood something while reading that thread.
See recent discussions on "QOM path stability":
https://lore.kernel.org/qemu-devel/zzfyvlmcxbcia...@redhat.com/
https://lore.kernel.org/qemu-devel/87jzojbxt7@pond.sub.org/
https://lore.kernel.org/qemu-devel/87v883by34@pond.sub.org/
If I read it right, the commit 46f7afa37096 example is pretty special that
the QOM path more or less decided more than the hierachy itself but changes
the existances of objects.
Let's see whether I got this...
We removed some useless objects, moved the useful ones to another home.
The move changed their QOM path.
The problem was the removal of useless objects, because this also
removed their vmstate.
If you checkout at the removal commit (5bc8d26de20c), the vmstate has
been kept untouched.
The fix was adding the vmstate back as a dummy.
Since the vmstate was kept I don't see why would we need a dummy. The
incoming migration stream would still have the state, only at a
different point in the stream. It's surprising to me that that would
cause an issue, but I'm not well versed in that code.
The QOM patch changes are *not* part of the problem.
The only explanation I can come up with is that after the patch
migration has broken after a hotplug or similar operation. In such
situation, the preallocated state would always be present before the
patch, but sometimes not present after the
[PATCH v3 3/9] hw/pci-host/astro: Add missing astro & elroy registers for NetBSD
From: Helge Deller
NetBSD accesses some astro and elroy registers which aren't accessed
by Linux yet. Add emulation for those registers to allow NetBSD to
boot further.
Please note that this patch is not sufficient to completely boot up
NetBSD on the 64-bit C3700 machine yet.
Signed-off-by: Helge Deller
Tested-by: Bruno Haible
---
hw/pci-host/astro.c | 26 +++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/hw/pci-host/astro.c b/hw/pci-host/astro.c
index 7d68ccee7e..cb2c8a828d 100644
--- a/hw/pci-host/astro.c
+++ b/hw/pci-host/astro.c
@@ -166,6 +166,8 @@ static MemTxResult elroy_chip_write_with_attrs(void
*opaque, hwaddr addr,
trace_elroy_write(addr, size, val);
switch ((addr >> 3) << 3) {
+case 0x000: /* PCI_ID & PCI_COMMAND_STATUS_REG */
+break;
case 0x080:
put_val_in_int64(&s->arb_mask, addr, size, val);
break;
@@ -175,6 +177,9 @@ static MemTxResult elroy_chip_write_with_attrs(void
*opaque, hwaddr addr,
case 0x200 ... 0x250 - 1: /* LMMIO, GMMIO, WLMMIO, WGMMIO, ... */
put_val_in_arrary(s->mmio_base, 0x200, addr, size, val);
break;
+case 0x300: /* ibase */
+case 0x308: /* imask */
+break;
case 0x0680:
put_val_in_int64(&s->error_config, addr, size, val);
break;
@@ -538,6 +543,9 @@ static MemTxResult astro_chip_read_with_attrs(void *opaque,
hwaddr addr,
case 0x0030:/* HP-UX 10.20 and 11.11 reads it. No idea. */
val = -1;
break;
+case 0x0078:/* NetBSD reads 0x78 ? */
+val = -1;
+break;
case 0x0300 ... 0x03d8: /* LMMIO_DIRECT0_BASE... */
index = (addr - 0x300) / 8;
val = s->ioc_ranges[index];
@@ -624,31 +632,43 @@ static MemTxResult astro_chip_write_with_attrs(void
*opaque, hwaddr addr,
case 0x10220:
case 0x10230:/* HP-UX 11.11 reads it. No idea. */
break;
-case 0x22108:/* IOC STATUS_CONTROL */
-put_val_in_int64(&s->ioc_status_ctrl, addr, size, val);
-break;
case 0x20200 ... 0x20240 - 1: /* IOC Rope0_Control ... */
put_val_in_arrary(s->ioc_rope_control, 0x20200, addr, size, val);
break;
case 0x20040:/* IOC Rope config */
+case 0x22040:
put_val_in_int64(&s->ioc_rope_config, addr, size, val);
break;
case 0x20300:
+case 0x22300:
put_val_in_int64(&s->tlb_ibase, addr, size, val);
break;
case 0x20308:
+case 0x22308:
put_val_in_int64(&s->tlb_imask, addr, size, val);
break;
case 0x20310:
+case 0x22310:
put_val_in_int64(&s->tlb_pcom, addr, size, val);
/* TODO: flush iommu */
break;
case 0x20318:
+case 0x22318:
put_val_in_int64(&s->tlb_tcnfg, addr, size, val);
break;
case 0x20320:
+case 0x22320:
put_val_in_int64(&s->tlb_pdir_base, addr, size, val);
break;
+case 0x22000: /* func_id */
+break;
+case 0x22008: /* func_class */
+break;
+case 0x22050: /* rope_debug */
+break;
+case 0x22108:/* IOC STATUS_CONTROL */
+put_val_in_int64(&s->ioc_status_ctrl, addr, size, val);
+break;
/*
* empty placeholders for non-existent elroys, e.g.
* func_class, pci config & data
--
2.43.0
[PATCH v3 6/9] target/hppa: Avoid accessing %gr0 when raising exception
From: Helge Deller The value of unwind_breg may reference register %r0, but we need to avoid accessing gr0 directly and use the value 0 instead. At runtime I've seen unwind_breg being zero with the Linux kernel when rfi is used to jump to smp_callin(). Signed-off-by: Helge Deller Reviewed-by: Richard Henderson Tested-by: Bruno Haible --- target/hppa/mem_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c index 011b192406..42bd0063c0 100644 --- a/target/hppa/mem_helper.c +++ b/target/hppa/mem_helper.c @@ -335,7 +335,7 @@ raise_exception_with_ior(CPUHPPAState *env, int excp, uintptr_t retaddr, cpu_restore_state(cs, retaddr); -b = env->gr[env->unwind_breg]; +b = env->unwind_breg ? env->gr[env->unwind_breg] : 0; b >>= (env->psw & PSW_W ? 62 : 30); env->cr[CR_IOR] |= b << 62; -- 2.43.0
[PATCH v3 8/9] target/hppa: Fix IOR and ISR on unaligned access trap
From: Helge Deller
Put correct values (depending on CPU arch) into IOR and ISR on fault.
Signed-off-by: Helge Deller
---
target/hppa/cpu.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/target/hppa/cpu.c b/target/hppa/cpu.c
index 04de1689d7..fda32d7f59 100644
--- a/target/hppa/cpu.c
+++ b/target/hppa/cpu.c
@@ -110,11 +110,7 @@ void hppa_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
CPUHPPAState *env = &cpu->env;
cs->exception_index = EXCP_UNALIGN;
-if (env->psw & PSW_Q) {
-/* ??? Needs tweaking for hppa64. */
-env->cr[CR_IOR] = addr;
-env->cr[CR_ISR] = addr >> 32;
-}
+hppa_set_ior_and_isr(env, addr, MMU_IDX_MMU_DISABLED(mmu_idx));
cpu_loop_exit_restore(cs, retaddr);
}
--
2.43.0
[PATCH v3 1/9] hw/hppa/machine: Allow up to 3840 MB total memory
From: Helge Deller
The physical hardware allows DIMMs of 4 MB size and above, allowing up
to 3840 MB of memory, but is restricted by setup code to 3 GB.
Increase the limit to allow up to the maximum amount of memory.
Btw. the memory area from 0xf000. to 0x. is reserved by
the architecture for firmware and I/O memory and can not be used for
standard memory.
An upcoming 64-bit SeaBIOS-hppa firmware will allow more than 3.75GB
on 64-bit HPPA64. In this case the ram_max for the pa20 case will change.
Signed-off-by: Helge Deller
Noticed-by: Nelson H. F. Beebe
Fixes: b7746b1194c8 ("hw/hppa/machine: Restrict the total memory size to 3GB")
Reviewed-by: Richard Henderson
Tested-by: Bruno Haible
---
hw/hppa/machine.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index c8da7c18d5..b11907617e 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -276,6 +276,7 @@ static TranslateFn
*machine_HP_common_init_cpus(MachineState *machine)
unsigned int smp_cpus = machine->smp.cpus;
TranslateFn *translate;
MemoryRegion *cpu_region;
+uint64_t ram_max;
/* Create CPUs. */
for (unsigned int i = 0; i < smp_cpus; i++) {
@@ -288,8 +289,10 @@ static TranslateFn
*machine_HP_common_init_cpus(MachineState *machine)
*/
if (hppa_is_pa20(&cpu[0]->env)) {
translate = translate_pa20;
+ram_max = 0xf000; /* 3.75 GB (limited by 32-bit firmware) */
} else {
translate = translate_pa10;
+ram_max = 0xf000; /* 3.75 GB (32-bit CPU) */
}
for (unsigned int i = 0; i < smp_cpus; i++) {
@@ -311,9 +314,9 @@ static TranslateFn
*machine_HP_common_init_cpus(MachineState *machine)
cpu_region);
/* Main memory region. */
-if (machine->ram_size > 3 * GiB) {
-error_report("RAM size is currently restricted to 3GB");
-exit(EXIT_FAILURE);
+if (machine->ram_size > ram_max) {
+info_report("Max RAM size limited to %" PRIu64 " MB", ram_max / MiB);
+machine->ram_size = ram_max;
}
memory_region_add_subregion_overlap(addr_space, 0, machine->ram, -1);
--
2.43.0
[PATCH v3 0/9] target/hppa qemu v8.2 regression fixes
From: Helge Deller There were some regressions introduced with Qemu v8.2 on the hppa/hppa64 target, e.g.: - 32-bit HP-UX crashes on B160L (32-bit) machine - NetBSD boot failure due to power button in page zero - NetBSD FPU detection failure - OpenBSD 7.4 boot failure This patch series fixes those known regressions and additionally: - allows usage of the max. 3840MB of memory (instead of 3GB), - adds support for the qemu --nodefaults option (to debug other devices) This patch set will not fix those known (non-regression) bugs: - HP-UX and NetBSD still fail to boot on the new 64-bit C3700 machine - Linux kernel will still fail to boot on C3700 as long as kernel modules are used. The whole series can be pulled from the "hppa-fixes-8.2" branch from: https://github.com/hdeller/qemu-hppa.githppa-fixes-8.2 SeaBIOS v15 has not changed in v3, so it is not included in this patch review series but will be added in final pull request. Please review. Changes v2->v3: - Added comment about Figures H-10 and H-11 in the parisc2.0 spec in patch which calculate PDC address translation if PSW.W=0 - Introduce and use hppa_set_ior_and_isr() - Use drive_get_max_bus(IF_SCSI), nd_table[] and serial_hd() to check if default devices should be created - Added Tested-by and Reviewed-by tags Changes v1->v2: - fix OpenBSD boot with SeaBIOS v15 instead of v14 - commit message enhancements suggested by BALATON Zoltan - use uint64_t for ram_max in patch #1 Helge Deller (9): hw/hppa/machine: Allow up to 3840 MB total memory hw/hppa/machine: Disable default devices with --nodefaults option hw/pci-host/astro: Add missing astro & elroy registers for NetBSD target/hppa: Fix PDC address translation on PA2.0 with PSW.W=0 hw/hppa: Move software power button address back into PDC target/hppa: Avoid accessing %gr0 when raising exception target/hppa: Export function hppa_set_ior_and_isr() target/hppa: Fix IOR and ISR on unaligned access trap target/hppa: Fix IOR and ISR on error in probe hw/hppa/machine.c| 28 +--- hw/pci-host/astro.c | 26 +++--- target/hppa/cpu.c| 6 +- target/hppa/cpu.h| 1 + target/hppa/mem_helper.c | 27 ++- target/hppa/op_helper.c | 6 +- 6 files changed, 57 insertions(+), 37 deletions(-) -- 2.43.0
[PATCH v3 2/9] hw/hppa/machine: Disable default devices with --nodefaults option
From: Helge Deller
Recognize the qemu --nodefaults option, which will disable the
following default devices on hppa:
- lsi53c895a SCSI controller,
- artist graphics card,
- LASI 82596 NIC,
- tulip PCI NIC,
- second serial PCI card,
- USB OHCI controller.
Adding this option is very useful to allow manual testing and
debugging of the other possible devices on the command line.
Signed-off-by: Helge Deller
---
hw/hppa/machine.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index b11907617e..54ca2fd91a 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -346,8 +346,10 @@ static void machine_HP_common_init_tail(MachineState
*machine, PCIBus *pci_bus,
SysBusDevice *s;
/* SCSI disk setup. */
-dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
-lsi53c8xx_handle_legacy_cmdline(dev);
+if (drive_get_max_bus(IF_SCSI) >= 0) {
+dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
+lsi53c8xx_handle_legacy_cmdline(dev);
+}
/* Graphics setup. */
if (machine->enable_graphics && vga_interface_type != VGA_NONE) {
@@ -360,7 +362,7 @@ static void machine_HP_common_init_tail(MachineState
*machine, PCIBus *pci_bus,
}
/* Network setup. */
-if (enable_lasi_lan()) {
+if (nd_table[0].used && enable_lasi_lan()) {
lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA),
qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA));
}
@@ -385,7 +387,7 @@ static void machine_HP_common_init_tail(MachineState
*machine, PCIBus *pci_bus,
pci_set_word(&pci_dev->config[PCI_SUBSYSTEM_ID], 0x1227); /* Powerbar */
/* create a second serial PCI card when running Astro */
-if (!lasi_dev) {
+if (serial_hd(1) && !lasi_dev) {
pci_dev = pci_new(-1, "pci-serial-4x");
qdev_prop_set_chr(DEVICE(pci_dev), "chardev1", serial_hd(1));
qdev_prop_set_chr(DEVICE(pci_dev), "chardev2", serial_hd(2));
--
2.43.0
[PATCH v3 7/9] target/hppa: Export function hppa_set_ior_and_isr()
From: Helge Deller
Move functionality to set IOR and ISR on fault into own
function. This will be used by follow-up patches.
Signed-off-by: Helge Deller
---
target/hppa/cpu.h| 1 +
target/hppa/mem_helper.c | 23 ---
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 8be45c69c9..9556e95fab 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -385,6 +385,7 @@ void hppa_cpu_dump_state(CPUState *cs, FILE *f, int);
#ifndef CONFIG_USER_ONLY
void hppa_ptlbe(CPUHPPAState *env);
hwaddr hppa_cpu_get_phys_page_debug(CPUState *cs, vaddr addr);
+void hppa_set_ior_and_isr(CPUHPPAState *env, vaddr addr, bool mmu_disabled);
bool hppa_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr);
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 42bd0063c0..7f54160e4c 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -299,14 +299,8 @@ hwaddr hppa_cpu_get_phys_page_debug(CPUState *cs, vaddr
addr)
return excp == EXCP_DTLB_MISS ? -1 : phys;
}
-G_NORETURN static void
-raise_exception_with_ior(CPUHPPAState *env, int excp, uintptr_t retaddr,
- vaddr addr, bool mmu_disabled)
+void hppa_set_ior_and_isr(CPUHPPAState *env, vaddr addr, bool mmu_disabled)
{
-CPUState *cs = env_cpu(env);
-
-cs->exception_index = excp;
-
if (env->psw & PSW_Q) {
/*
* For pa1.x, the offset and space never overlap, and so we
@@ -333,16 +327,23 @@ raise_exception_with_ior(CPUHPPAState *env, int excp,
uintptr_t retaddr,
*/
uint64_t b;
-cpu_restore_state(cs, retaddr);
-
b = env->unwind_breg ? env->gr[env->unwind_breg] : 0;
b >>= (env->psw & PSW_W ? 62 : 30);
env->cr[CR_IOR] |= b << 62;
-
-cpu_loop_exit(cs);
}
}
}
+}
+
+G_NORETURN static void
+raise_exception_with_ior(CPUHPPAState *env, int excp, uintptr_t retaddr,
+ vaddr addr, bool mmu_disabled)
+{
+CPUState *cs = env_cpu(env);
+
+cs->exception_index = excp;
+hppa_set_ior_and_isr(env, addr, mmu_disabled);
+
cpu_loop_exit_restore(cs, retaddr);
}
--
2.43.0
[PATCH v3 5/9] hw/hppa: Move software power button address back into PDC
From: Helge Deller
The various operating systems (e.g. Linux, NetBSD) have issues
mapping the power button when it's stored in page zero.
NetBSD even crashes, because it fails to map that page and then
accesses unmapped memory.
Since we now have a consistent memory mapping of PDC in 32-bit
and 64-bit address space (the lower 32-bits of the address are in
sync) the power button can be moved back to PDC space.
This patch fixes the power button on Linux, NetBSD and HP-UX.
Signed-off-by: Helge Deller
Tested-by: Bruno Haible
---
hw/hppa/machine.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 54ca2fd91a..9e611620cc 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -36,8 +36,8 @@
#define MIN_SEABIOS_HPPA_VERSION 12 /* require at least this fw version */
-/* Power button address at &PAGE0->pad[4] */
-#define HPA_POWER_BUTTON (0x40 + 4 * sizeof(uint32_t))
+#define HPA_POWER_BUTTON(FIRMWARE_END - 0x10)
+static hwaddr soft_power_reg;
#define enable_lasi_lan() 0
@@ -45,7 +45,6 @@ static DeviceState *lasi_dev;
static void hppa_powerdown_req(Notifier *n, void *opaque)
{
-hwaddr soft_power_reg = HPA_POWER_BUTTON;
uint32_t val;
val = ldl_be_phys(&address_space_memory, soft_power_reg);
@@ -221,7 +220,7 @@ static FWCfgState *create_fw_cfg(MachineState *ms, PCIBus
*pci_bus,
fw_cfg_add_file(fw_cfg, "/etc/hppa/machine",
g_memdup(mc->name, len), len);
-val = cpu_to_le64(HPA_POWER_BUTTON);
+val = cpu_to_le64(soft_power_reg);
fw_cfg_add_file(fw_cfg, "/etc/hppa/power-button-addr",
g_memdup(&val, sizeof(val)), sizeof(val));
@@ -295,6 +294,8 @@ static TranslateFn
*machine_HP_common_init_cpus(MachineState *machine)
ram_max = 0xf000; /* 3.75 GB (32-bit CPU) */
}
+soft_power_reg = translate(NULL, HPA_POWER_BUTTON);
+
for (unsigned int i = 0; i < smp_cpus; i++) {
g_autofree char *name = g_strdup_printf("cpu%u-io-eir", i);
--
2.43.0
[PATCH v3 9/9] target/hppa: Fix IOR and ISR on error in probe
From: Helge Deller
Put correct values (depending on CPU arch) into IOR and ISR on fault.
Signed-off-by: Helge Deller
---
target/hppa/op_helper.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
index 7f607c3afd..ce15469465 100644
--- a/target/hppa/op_helper.c
+++ b/target/hppa/op_helper.c
@@ -351,11 +351,7 @@ target_ulong HELPER(probe)(CPUHPPAState *env, target_ulong
addr,
excp = hppa_get_physical_address(env, addr, mmu_idx, 0, &phys,
&prot, NULL);
if (excp >= 0) {
-if (env->psw & PSW_Q) {
-/* ??? Needs tweaking for hppa64. */
-env->cr[CR_IOR] = addr;
-env->cr[CR_ISR] = addr >> 32;
-}
+hppa_set_ior_and_isr(env, addr, MMU_IDX_MMU_DISABLED(mmu_idx));
if (excp == EXCP_DTLB_MISS) {
excp = EXCP_NA_DTLB_MISS;
}
--
2.43.0
[PATCH v3 4/9] target/hppa: Fix PDC address translation on PA2.0 with PSW.W=0
From: Helge Deller
Fix the address translation for PDC space on PA2.0 if PSW.W=0.
Basically, for any address in the 32-bit PDC range from 0xf000 to
0xf100 keep the lower 32-bits and just set the upper 32-bits to
0xfff0.
This mapping fixes the emulated power button in PDC space for 32- and
64-bit machines and is how the physical C3700 machine seems to map
PDC.
Figures H-10 and H-11 in the parisc2.0 spec [1] show that the 32-bit
region will be mapped somewhere into a higher and bigger 64-bit PDC
space. The start and end of this 64-bit space is defined by the
physical address bits. But the figures don't specifiy where exactly the
mapping will start inside that region. Tests on a real HP C3700
regarding the address of the power button indicate, that the lower
32-bits will stay the same though.
[1] https://parisc.wiki.kernel.org/images-parisc/7/73/Parisc2.0.pdf
Signed-off-by: Helge Deller
Tested-by: Bruno Haible
---
target/hppa/mem_helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 08abd1a9f9..011b192406 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -56,7 +56,7 @@ hwaddr hppa_abs_to_phys_pa2_w0(vaddr addr)
addr = (int32_t)addr;
} else {
/* PDC address space */
-addr &= MAKE_64BIT_MASK(0, 24);
+addr = (uint32_t)addr;
addr |= -1ull << (TARGET_PHYS_ADDR_SPACE_BITS - 4);
}
return addr;
--
2.43.0
[PATCH] coroutine-ucontext: Save fake stack for pooled coroutine
Coroutine may be pooled even after COROUTINE_TERMINATE if
CONFIG_COROUTINE_POOL is enabled and fake stack should be saved in
such a case to keep AddressSanitizerUseAfterReturn working. Even worse,
I'm seeing stack corruption without fake stack being saved.
Signed-off-by: Akihiko Odaki
---
util/coroutine-ucontext.c | 21 +++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index 7b304c79d942..e62ced5d6779 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -124,8 +124,9 @@ void start_switch_fiber_asan(CoroutineAction action, void
**fake_stack_save,
{
#ifdef CONFIG_ASAN
__sanitizer_start_switch_fiber(
-action == COROUTINE_TERMINATE ? NULL : fake_stack_save,
-bottom, size);
+!IS_ENABLED(CONFIG_COROUTINE_POOL) && action == COROUTINE_TERMINATE ?
+NULL : fake_stack_save,
+bottom, size);
#endif
}
@@ -269,10 +270,26 @@ static inline void
valgrind_stack_deregister(CoroutineUContext *co)
#endif
#endif
+#if defined(CONFIG_ASAN) && defined(CONFIG_COROUTINE_POOL)
+static void coroutine_fn terminate(void *opaque)
+{
+CoroutineUContext *to = DO_UPCAST(CoroutineUContext, base, opaque);
+
+__sanitizer_start_switch_fiber(NULL, to->stack, to->stack_size);
+siglongjmp(to->env, COROUTINE_ENTER);
+}
+#endif
+
void qemu_coroutine_delete(Coroutine *co_)
{
CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
+#if defined(CONFIG_ASAN) && defined(CONFIG_COROUTINE_POOL)
+co_->entry_arg = qemu_coroutine_self();
+co_->entry = terminate;
+qemu_coroutine_switch(co_->entry_arg, co_, COROUTINE_ENTER);
+#endif
+
#ifdef CONFIG_VALGRIND_H
valgrind_stack_deregister(co);
#endif
---
base-commit: f614acb7450282a119d85d759f27eae190476058
change-id: 20240112-asan-eb695c769f40
Best regards,
--
Akihiko Odaki
Re: [RFC PATCH v3 04/30] io: fsync before closing a file channel
On Fri, Jan 12, 2024 at 08:01:36AM +0800, Peter Xu wrote: > On Thu, Jan 11, 2024 at 03:46:02PM -0300, Fabiano Rosas wrote: > > > (1) Does this apply to all io channel users, or only migration? > > > > All file channel users. > > I meant the whole idea of flushing on close, on whether there will be > iochannel users that will prefer not do so? It's a matter of where to put > this best. IMHO, all users of QIOChannelFile will benefit from flushing, to ensure integrity of their saved file upon host crash. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH 1/2] target/ppc/cpu-models: Rename power5+ and power7+ for new QOM naming rules
On 12/01/2024 06.21, Harsh Prateek Bora wrote:
On 1/12/24 10:42, Thomas Huth wrote:
On 12/01/2024 05.57, Harsh Prateek Bora wrote:
On 1/11/24 22:16, Thomas Huth wrote:
The character "+" is now forbidden in QOM device names (see commit
b447378e1217 - "Limit type names to alphanumerical and some few special
characters"). For the "power5+" and "power7+" CPU names, there is
currently a hack in type_name_is_valid() to still allow them for
compatibility reasons. However, there is a much nicer solution for this:
Simply use aliases! This way we can still support the old names without
the need for the ugly hack in type_name_is_valid().
Signed-off-by: Thomas Huth
---
hw/ppc/spapr_cpu_core.c | 4 ++--
qom/object.c | 4
target/ppc/cpu-models.c | 10 ++
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 5aa1ed474a..214b7a03d8 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -389,9 +389,9 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
DEFINE_SPAPR_CPU_CORE_TYPE("970_v2.2"),
DEFINE_SPAPR_CPU_CORE_TYPE("970mp_v1.0"),
DEFINE_SPAPR_CPU_CORE_TYPE("970mp_v1.1"),
- DEFINE_SPAPR_CPU_CORE_TYPE("power5+_v2.1"),
+ DEFINE_SPAPR_CPU_CORE_TYPE("power5plus_v2.1"),
DEFINE_SPAPR_CPU_CORE_TYPE("power7_v2.3"),
- DEFINE_SPAPR_CPU_CORE_TYPE("power7+_v2.1"),
+ DEFINE_SPAPR_CPU_CORE_TYPE("power7plus_v2.1"),
Will using Power5x, Power7x be a better naming than using 'plus' suffix ?
The "x" looks like a placeholder to me, so it could be confused with
power50, power51, power52, etc. ...?
But actually, I was thinking about using "power5p" and "power7p" first, so
if the whole "plus" looks too long for you, would "p" be an option instead?
Hmm .. I would certainly vote for 'p' over 'plus'.
Ok, I don't mind either way ... does anybody else have any preferences?
Thomas
[PULL 02/22] tests/avocado: use snapshot=on in kvm_xen_guest
This ensures the rootfs is never permanently changed as we don't need
persistence between tests anyway.
Message-Id: <20240103173349.398526-3-alex.ben...@linaro.org>
Signed-off-by: Alex Bennée
Reviewed-by: Philippe Mathieu-Daudé
diff --git a/tests/avocado/kvm_xen_guest.py b/tests/avocado/kvm_xen_guest.py
index 5391283113e..f8cb458d5db 100644
--- a/tests/avocado/kvm_xen_guest.py
+++ b/tests/avocado/kvm_xen_guest.py
@@ -59,7 +59,7 @@ def common_vm_setup(self):
def run_and_check(self):
self.vm.add_args('-kernel', self.kernel_path,
'-append', self.kernel_params,
- '-drive',
f"file={self.rootfs},if=none,format=raw,id=drv0",
+ '-drive',
f"file={self.rootfs},if=none,snapshot=on,format=raw,id=drv0",
'-device', 'xen-disk,drive=drv0,vdev=xvda',
'-device', 'virtio-net-pci,netdev=unet',
'-netdev', 'user,id=unet,hostfwd=:127.0.0.1:0-:22')
--
2.39.2
[PULL 16/22] tests/qtest: Bump the device-introspect-test timeout to 12 minutes
From: Thomas Huth
When running the test in slow mode on a very loaded system with the
arm/aarch64 target and with --enable-debug, it can take longer than
10 minutes to finish the introspection test. Bump the timeout to twelve
minutes to make sure that it also finishes in such situations.
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-13-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 9e0ad15dfc9..fd40136fa9c 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -1,6 +1,7 @@
slow_qtests = {
'aspeed_smc-test': 360,
'bios-tables-test' : 540,
+ 'device-introspect-test' : 720,
'migration-test' : 480,
'npcm7xx_pwm-test': 300,
'qom-test' : 900,
--
2.39.2
[PULL 03/22] gitlab: include microblazeel in testing
This reverts aeb5f8f248e (gitlab: build the correct microblaze target) now we actually have a little-endian test in avocado thanks to this years advent calendar. Message-Id: <20240103173349.398526-4-alex.ben...@linaro.org> Signed-off-by: Alex Bennée Reviewed-by: Thomas Huth diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index 91663946de4..ef71dfe8665 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -41,7 +41,7 @@ build-system-ubuntu: variables: IMAGE: ubuntu2204 CONFIGURE_ARGS: --enable-docs -TARGETS: alpha-softmmu microblaze-softmmu mips64el-softmmu +TARGETS: alpha-softmmu microblazeel-softmmu mips64el-softmmu MAKE_CHECK_ARGS: check-build check-system-ubuntu: @@ -61,7 +61,7 @@ avocado-system-ubuntu: variables: IMAGE: ubuntu2204 MAKE_CHECK_ARGS: check-avocado -AVOCADO_TAGS: arch:alpha arch:microblaze arch:mips64el +AVOCADO_TAGS: arch:alpha arch:microblazeel arch:mips64el build-system-debian: extends: -- 2.39.2
[PULL 13/22] qtest: bump qos-test timeout to 2 minutes
From: Daniel P. Berrangé
The qos-test takes just under 1 minute in a --enable-debug
build. Bumping to 2 minutes will give more headroom.
Signed-off-by: Daniel P. Berrangé
Reviewed-by: Thomas Huth
Message-ID: <20230717182859.707658-10-berra...@redhat.com>
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-10-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 6d89bac722b..eaa2cf64a3f 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -7,6 +7,7 @@ slow_qtests = {
'pxe-test': 600,
'prom-env-test': 360,
'boot-serial-test': 180,
+ 'qos-test': 120,
}
qtests_generic = [
--
2.39.2
[PULL 08/22] qtest: bump npcm7xx_pwm-test timeout to 5 minutes
From: Daniel P. Berrangé
The npcm7xx_pwm-test takes 3 & 1/2 minutes in a --enable-debug build.
Bumping to 5 minutes will give more headroom.
Signed-off-by: Daniel P. Berrangé
Reviewed-by: Thomas Huth
Message-ID: <20230717182859.707658-5-berra...@redhat.com>
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-5-th...@redhat.com>
[AJB: s/pwn/pwm]
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index bb9d599e4dc..823a8ac38d0 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -1,7 +1,7 @@
slow_qtests = {
'bios-tables-test' : 120,
'migration-test' : 480,
- 'npcm7xx_pwm-test': 150,
+ 'npcm7xx_pwm-test': 300,
'qom-test' : 900,
'test-hmp' : 120,
}
--
2.39.2
[PULL 04/22] chardev: use bool for fe_is_open
The function qemu_chr_fe_init already treats be->fe_open as a bool and
if it acts like a bool it should be one. While we are at it make the
variable name more descriptive and add kdoc decorations.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Alex Bennée
Message-Id: <20231211145959.93759-1-alex.ben...@linaro.org>
diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h
index 0ff6f875116..ecef1828355 100644
--- a/include/chardev/char-fe.h
+++ b/include/chardev/char-fe.h
@@ -7,8 +7,12 @@
typedef void IOEventHandler(void *opaque, QEMUChrEvent event);
typedef int BackendChangeHandler(void *opaque);
-/* This is the backend as seen by frontend, the actual backend is
- * Chardev */
+/**
+ * struct CharBackend - back end as seen by front end
+ * @fe_is_open: the front end is ready for IO
+ *
+ * The actual backend is Chardev
+ */
struct CharBackend {
Chardev *chr;
IOEventHandler *chr_event;
@@ -17,7 +21,7 @@ struct CharBackend {
BackendChangeHandler *chr_be_change;
void *opaque;
int tag;
-int fe_open;
+bool fe_is_open;
};
/**
@@ -156,12 +160,13 @@ void qemu_chr_fe_set_echo(CharBackend *be, bool echo);
/**
* qemu_chr_fe_set_open:
+ * @be: a CharBackend
+ * @is_open: the front end open status
*
- * Set character frontend open status. This is an indication that the
- * front end is ready (or not) to begin doing I/O.
- * Without associated Chardev, do nothing.
+ * This is an indication that the front end is ready (or not) to begin
+ * doing I/O. Without associated Chardev, do nothing.
*/
-void qemu_chr_fe_set_open(CharBackend *be, int fe_open);
+void qemu_chr_fe_set_open(CharBackend *be, bool is_open);
/**
* qemu_chr_fe_printf:
diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 7789f7be9c8..20222a4cad5 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -211,7 +211,7 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error
**errp)
}
}
-b->fe_open = false;
+b->fe_is_open = false;
b->tag = tag;
b->chr = s;
return true;
@@ -257,7 +257,7 @@ void qemu_chr_fe_set_handlers_full(CharBackend *b,
bool sync_state)
{
Chardev *s;
-int fe_open;
+bool fe_open;
s = b->chr;
if (!s) {
@@ -265,10 +265,10 @@ void qemu_chr_fe_set_handlers_full(CharBackend *b,
}
if (!opaque && !fd_can_read && !fd_read && !fd_event) {
-fe_open = 0;
+fe_open = false;
remove_fd_in_watch(s);
} else {
-fe_open = 1;
+fe_open = true;
}
b->chr_can_read = fd_can_read;
b->chr_read = fd_read;
@@ -336,7 +336,7 @@ void qemu_chr_fe_set_echo(CharBackend *be, bool echo)
}
}
-void qemu_chr_fe_set_open(CharBackend *be, int fe_open)
+void qemu_chr_fe_set_open(CharBackend *be, bool is_open)
{
Chardev *chr = be->chr;
@@ -344,12 +344,12 @@ void qemu_chr_fe_set_open(CharBackend *be, int fe_open)
return;
}
-if (be->fe_open == fe_open) {
+if (be->fe_is_open == is_open) {
return;
}
-be->fe_open = fe_open;
+be->fe_is_open = is_open;
if (CHARDEV_GET_CLASS(chr)->chr_set_fe_open) {
-CHARDEV_GET_CLASS(chr)->chr_set_fe_open(chr, fe_open);
+CHARDEV_GET_CLASS(chr)->chr_set_fe_open(chr, is_open);
}
}
diff --git a/chardev/char.c b/chardev/char.c
index 185b5ea2556..3c43fb1278f 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -762,7 +762,7 @@ static int qmp_query_chardev_foreach(Object *obj, void
*data)
value->label = g_strdup(chr->label);
value->filename = g_strdup(chr->filename);
-value->frontend_open = chr->be && chr->be->fe_open;
+value->frontend_open = chr->be && chr->be->fe_is_open;
QAPI_LIST_PREPEND(*list, value);
--
2.39.2
[PULL 15/22] qtest: bump bios-table-test timeout to 9 minutes
From: Daniel P. Berrangé
This is reliably hitting the current 2 minute timeout in GitLab CI,
and for the TCI job, it even hits a 6 minute timeout.
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
Message-ID: <20230717182859.707658-12-berra...@redhat.com>
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-12-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c86a997b954..9e0ad15dfc9 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -1,6 +1,6 @@
slow_qtests = {
'aspeed_smc-test': 360,
- 'bios-tables-test' : 120,
+ 'bios-tables-test' : 540,
'migration-test' : 480,
'npcm7xx_pwm-test': 300,
'qom-test' : 900,
--
2.39.2
[PULL 01/22] tests/avocado: Add a test for a little-endian microblaze machine
From: Thomas Huth
We've already got a test for a big endian microblaze machine, but so
far we lack one for a little endian machine. Now that the QEMU advent
calendar featured such an image, we can test the little endian mode,
too.
Signed-off-by: Thomas Huth
Message-Id: <20231215161851.71508-1-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/avocado/machine_microblaze.py
b/tests/avocado/machine_microblaze.py
index 8d0efff30d2..807709cd11e 100644
--- a/tests/avocado/machine_microblaze.py
+++ b/tests/avocado/machine_microblaze.py
@@ -5,6 +5,8 @@
# This work is licensed under the terms of the GNU GPL, version 2 or
# later. See the COPYING file in the top-level directory.
+import time
+from avocado_qemu import exec_command, exec_command_and_wait_for_pattern
from avocado_qemu import QemuSystemTest
from avocado_qemu import wait_for_console_pattern
from avocado.utils import archive
@@ -33,3 +35,27 @@ def test_microblaze_s3adsp1800(self):
# The kernel sometimes gets stuck after the "This architecture ..."
# message, that's why we don't test for a later string here. This
# needs some investigation by a microblaze wizard one day...
+
+def test_microblazeel_s3adsp1800(self):
+"""
+:avocado: tags=arch:microblazeel
+:avocado: tags=machine:petalogix-s3adsp1800
+"""
+
+self.require_netdev('user')
+tar_url = ('http://www.qemu-advent-calendar.org/2023/download/'
+ 'day13.tar.gz')
+tar_hash = '6623d5fff5f84cfa8f34e286f32eff6a26546f44'
+file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
+archive.extract(file_path, self.workdir)
+self.vm.set_console()
+self.vm.add_args('-kernel', self.workdir + '/day13/xmaton.bin')
+self.vm.add_args('-nic', 'user,tftp=' + self.workdir + '/day13/')
+self.vm.launch()
+wait_for_console_pattern(self, 'QEMU Advent Calendar 2023')
+time.sleep(0.1)
+exec_command(self, 'root')
+time.sleep(0.1)
+exec_command_and_wait_for_pattern(self,
+'tftp -g -r xmaton.png 10.0.2.2 ; md5sum xmaton.png',
+'821cd3cab8efd16ad6ee5acc3642a8ea')
--
2.39.2
[PULL 06/22] qtest: bump migration-test timeout to 8 minutes
From: Daniel P. Berrangé
The migration test should take between 1 min 30 and 2 mins on reasonably
modern hardware. The test is not especially compute bound, rather its
running time is dominated by the guest RAM size relative to the
bandwidth cap, which forces each iteration to take at least 30 seconds.
None the less under high load conditions with multiple QEMU processes
spawned and competing with other parallel tests, the worst case running
time might be somewhat extended. Bumping the timeout to 8 minutes gives
us good headroom, while still catching stuck tests relatively quickly.
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
Message-ID: <20230717182859.707658-3-berra...@redhat.com>
[thuth: Bump timeout to 8 minutes to make it work on very loaded systems, too]
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-3-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index df013a36b32..2d2b37c2a78 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -1,6 +1,6 @@
slow_qtests = {
'bios-tables-test' : 120,
- 'migration-test' : 150,
+ 'migration-test' : 480,
'npcm7xx_pwm-test': 150,
'qom-test' : 300,
'test-hmp' : 120,
--
2.39.2
[PULL 14/22] qtest: bump aspeed_smc-test timeout to 6 minutes
From: Daniel P. Berrangé
On a loaded system with --enable-debug, this test can take longer than
5 minutes. Raising the timeout to 6 minutes gives greater headroom for
such situations.
Signed-off-by: Daniel P. Berrangé
[thuth: Increase the timeout to 6 minutes for very loaded systems]
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-11-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index eaa2cf64a3f..c86a997b954 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -1,4 +1,5 @@
slow_qtests = {
+ 'aspeed_smc-test': 360,
'bios-tables-test' : 120,
'migration-test' : 480,
'npcm7xx_pwm-test': 300,
--
2.39.2
[PULL 09/22] qtest: bump test-hmp timeout to 4 minutes
From: Daniel P. Berrangé
The hmp test takes just under 3 minutes in a --enable-debug
build. Bumping to 4 minutes will give more headroom.
Signed-off-by: Daniel P. Berrangé
Reviewed-by: Thomas Huth
Message-ID: <20230717182859.707658-6-berra...@redhat.com>
[thuth: fix copy-n-paste error in the description]
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-6-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 823a8ac38d0..3ea8279527c 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -3,7 +3,7 @@ slow_qtests = {
'migration-test' : 480,
'npcm7xx_pwm-test': 300,
'qom-test' : 900,
- 'test-hmp' : 120,
+ 'test-hmp' : 240,
}
qtests_generic = [
--
2.39.2
[PULL 05/22] qtest: bump min meson timeout to 60 seconds
From: Daniel P. Berrangé
Even some of the relatively fast qtests can sometimes hit the 30 second
timeout in GitLab CI under high parallelism/load conditions. Bump the
min to 60 seconds to give a higher margin for reliability.
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
Message-ID: <20230717182859.707658-2-berra...@redhat.com>
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-2-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index f25bffcc20a..df013a36b32 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -1,12 +1,7 @@
slow_qtests = {
- 'ahci-test' : 60,
'bios-tables-test' : 120,
- 'boot-serial-test' : 60,
'migration-test' : 150,
'npcm7xx_pwm-test': 150,
- 'prom-env-test' : 60,
- 'pxe-test' : 60,
- 'qos-test' : 60,
'qom-test' : 300,
'test-hmp' : 120,
}
@@ -383,8 +378,8 @@ foreach dir : target_dirs
env: qtest_env,
args: ['--tap', '-k'],
protocol: 'tap',
- timeout: slow_qtests.get(test, 30),
- priority: slow_qtests.get(test, 30),
+ timeout: slow_qtests.get(test, 60),
+ priority: slow_qtests.get(test, 60),
suite: ['qtest', 'qtest-' + target_base])
endforeach
endforeach
--
2.39.2
[PULL 10/22] qtest: bump pxe-test timeout to 10 minutes
From: Daniel P. Berrangé
The pxe-test uses the boot_sector_test() function, and that already
uses a timeout of 600 seconds. So adjust the timeout on the meson
side accordingly.
Signed-off-by: Daniel P. Berrangé
[thuth: Bump timeout to 600s and adjust commit description]
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-7-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 3ea8279527c..3f56b205181 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -4,6 +4,7 @@ slow_qtests = {
'npcm7xx_pwm-test': 300,
'qom-test' : 900,
'test-hmp' : 240,
+ 'pxe-test': 600,
}
qtests_generic = [
--
2.39.2
[PULL 00/22] testing and misc updates
The following changes since commit f614acb7450282a119d85d759f27eae190476058: Merge tag 'pull-target-arm-20240111' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-01-11 11:05:44 +) are available in the Git repository at: https://gitlab.com/stsquad/qemu.git tags/pull-testing-updates-120124-1 for you to fetch changes up to b4960adfbd8975579016c94a087eb2f6bece7eda: Revert "tests/avocado: remove skips from replay_kernel" (2024-01-12 10:08:16 +) testing and misc updates - add LE microblaze test to avocado - use modern snapshot=on to avoid trashing disk image - use plain bool for fe_is_open - various updates to qtest timeouts - enable meson test timeouts - tweak the readthedocs environment - revert the revert of flaky replay tests Alex Bennée (5): tests/avocado: use snapshot=on in kvm_xen_guest gitlab: include microblazeel in testing chardev: use bool for fe_is_open readthodocs: fully specify a build environment Revert "tests/avocado: remove skips from replay_kernel" Daniel P. Berrangé (12): qtest: bump min meson timeout to 60 seconds qtest: bump migration-test timeout to 8 minutes qtest: bump qom-test timeout to 15 minutes qtest: bump npcm7xx_pwm-test timeout to 5 minutes qtest: bump test-hmp timeout to 4 minutes qtest: bump pxe-test timeout to 10 minutes qtest: bump prom-env-test timeout to 6 minutes qtest: bump boot-serial-test timeout to 3 minutes qtest: bump qos-test timeout to 2 minutes qtest: bump aspeed_smc-test timeout to 6 minutes qtest: bump bios-table-test timeout to 9 minutes mtest2make: stop disabling meson test timeouts Thomas Huth (5): tests/avocado: Add a test for a little-endian microblaze machine tests/qtest: Bump the device-introspect-test timeout to 12 minutes tests/unit: Bump test-aio-multithread test timeout to 2 minutes tests/unit: Bump test-crypto-block test timeout to 5 minutes tests/fp: Bump fp-test-mulAdd test timeout to 3 minutes docs/requirements.txt | 2 ++ include/chardev/char-fe.h | 19 --- chardev/char-fe.c | 16 chardev/char.c | 2 +- .gitlab-ci.d/buildtest.yml | 4 ++-- .readthedocs.yml| 19 --- scripts/mtest2make.py | 3 ++- tests/avocado/kvm_xen_guest.py | 2 +- tests/avocado/machine_microblaze.py | 26 ++ tests/avocado/replay_kernel.py | 11 +++ tests/fp/meson.build| 2 +- tests/qtest/meson.build | 25 + tests/unit/meson.build | 2 ++ 13 files changed, 93 insertions(+), 40 deletions(-) create mode 100644 docs/requirements.txt -- 2.39.2
[PULL 11/22] qtest: bump prom-env-test timeout to 6 minutes
From: Daniel P. Berrangé
The prom-env-test can take more than 5 minutes in a --enable-debug
build on a loaded system. Bumping to 6 minutes will give more headroom.
Signed-off-by: Daniel P. Berrangé
[thuth: Bump timeout to 6 minutes instead of 3]
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-8-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 3f56b205181..b3b2e3857bd 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -5,6 +5,7 @@ slow_qtests = {
'qom-test' : 900,
'test-hmp' : 240,
'pxe-test': 600,
+ 'prom-env-test': 360,
}
qtests_generic = [
--
2.39.2
[PULL 12/22] qtest: bump boot-serial-test timeout to 3 minutes
From: Daniel P. Berrangé
The boot-serial-test takes about 1 + 1/2 minutes in a --enable-debug
build. Bumping to 3 minutes will give more headroom.
Signed-off-by: Daniel P. Berrangé
Reviewed-by: Thomas Huth
Message-ID: <20230717182859.707658-9-berra...@redhat.com>
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-9-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index b3b2e3857bd..6d89bac722b 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -6,6 +6,7 @@ slow_qtests = {
'test-hmp' : 240,
'pxe-test': 600,
'prom-env-test': 360,
+ 'boot-serial-test': 180,
}
qtests_generic = [
--
2.39.2
[PULL 07/22] qtest: bump qom-test timeout to 15 minutes
From: Daniel P. Berrangé
The qom-test is periodically hitting the 5 minute timeout when running
on the aarch64 emulator under GitLab CI. With an --enable-debug build
it can take over 10 minutes for arm/aarch64 targets. Setting timeout
to 15 minutes gives enough headroom to hopefully make it reliable.
Reviewed-by: Thomas Huth
Signed-off-by: Daniel P. Berrangé
Message-ID: <20230717182859.707658-4-berra...@redhat.com>
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-4-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 2d2b37c2a78..bb9d599e4dc 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -2,7 +2,7 @@ slow_qtests = {
'bios-tables-test' : 120,
'migration-test' : 480,
'npcm7xx_pwm-test': 150,
- 'qom-test' : 300,
+ 'qom-test' : 900,
'test-hmp' : 120,
}
--
2.39.2
[PULL 20/22] mtest2make: stop disabling meson test timeouts
From: Daniel P. Berrangé The mtest2make.py script passes the arg '-t 0' to 'meson test' which disables all test timeouts. This is a major source of pain when running in GitLab CI and a test gets stuck. It will stall until GitLab kills the CI job. This leaves us with little easily consumable information about the stalled test. The TAP format doesn't show the test name until it is completed, and TAP output from multiple tests it interleaved. So we have to analyse the log to figure out what tests had un-finished TAP output present and thus infer which test case caused the hang. This is very time consuming and error prone. By allowing meson to kill stalled tests, we get a direct display of what test program got stuck, which lets us more directly focus in on what specific test case within the test program hung. The other issue with disabling meson test timeouts by default is that it makes it more likely that maintainers inadvertantly introduce slowdowns. For example the recent-ish change that accidentally made migrate-test take 15-20 minutes instead of around 1 minute. The main risk of this change is that the individual test timeouts might be too short to allow completion in high load scenarios. Thus, there is likely to be some short term pain where we have to bump the timeouts for certain tests to make them reliable enough. The preceeding few patches raised the timeouts for all failures that were immediately apparent in GitLab CI. Even with the possible short term instability, this should still be a net win for debuggability of failed CI pipelines over the long term. Signed-off-by: Daniel P. Berrangé Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20230717182859.707658-13-berra...@redhat.com> Signed-off-by: Thomas Huth Message-Id: <20231215070357.10888-17-th...@redhat.com> Signed-off-by: Alex Bennée diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py index 179dd548718..eb01a05ddbd 100644 --- a/scripts/mtest2make.py +++ b/scripts/mtest2make.py @@ -27,7 +27,8 @@ def names(self, base): .speed.slow = $(foreach s,$(sort $(filter-out %-thorough, $1)), --suite $s) .speed.thorough = $(foreach s,$(sort $1), --suite $s) -.mtestargs = --no-rebuild -t 0 +TIMEOUT_MULTIPLIER = 1 +.mtestargs = --no-rebuild -t $(TIMEOUT_MULTIPLIER) ifneq ($(SPEED), quick) .mtestargs += --setup $(SPEED) endif -- 2.39.2
[PULL 17/22] tests/unit: Bump test-aio-multithread test timeout to 2 minutes
From: Thomas Huth
When running the tests in slow mode on a very loaded system and with
--enable-debug, the test-aio-multithread can take longer than 1 minute.
Bump the timeout to two minutes to make sure that it also passes in
such situations.
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-14-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 69f9c050504..937e1ebd356 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -172,6 +172,7 @@ test_env.set('G_TEST_SRCDIR', meson.current_source_dir())
test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
slow_tests = {
+ 'test-aio-multithread' : 120,
'test-crypto-tlscredsx509': 45,
'test-crypto-tlssession': 45
}
--
2.39.2
[PULL 22/22] Revert "tests/avocado: remove skips from replay_kernel"
This reverts commit c2ef5ee89d76f0ab77c4dd6a1c9eeed4d35d20ed.
While the fixes for #2010 and #2013 have improved things locally it
seems GitLab still continues to be flaky.
Signed-off-by: Alex Bennée
diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
index 6fdcbd6ac3d..1eaa36444cb 100644
--- a/tests/avocado/replay_kernel.py
+++ b/tests/avocado/replay_kernel.py
@@ -98,10 +98,13 @@ def test_i386_pc(self):
self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=5)
+# See https://gitlab.com/qemu-project/qemu/-/issues/2010
+@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test sometimes gets
stuck')
def test_x86_64_pc(self):
"""
:avocado: tags=arch:x86_64
:avocado: tags=machine:pc
+:avocado: tags=flaky
"""
kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
'/linux/releases/29/Everything/x86_64/os/images/pxeboot'
@@ -132,6 +135,8 @@ def test_mips_malta(self):
self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=5)
+# See https://gitlab.com/qemu-project/qemu/-/issues/2013
+@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on
GitLab')
def test_mips64el_malta(self):
"""
This test requires the ar tool to extract "data.tar.gz" from
@@ -147,6 +152,7 @@ def test_mips64el_malta(self):
:avocado: tags=arch:mips64el
:avocado: tags=machine:malta
+:avocado: tags=flaky
"""
deb_url = ('http://snapshot.debian.org/archive/debian/'
'20130217T032700Z/pool/main/l/linux-2.6/'
@@ -194,10 +200,13 @@ def test_arm_virt(self):
self.run_rr(kernel_path, kernel_command_line, console_pattern, shift=1)
+@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on
GitLab')
+
def test_arm_cubieboard_initrd(self):
"""
:avocado: tags=arch:arm
:avocado: tags=machine:cubieboard
+:avocado: tags=flaky
"""
deb_url = ('https://apt.armbian.com/pool/main/l/'
'linux-5.10.16-sunxi/linux-image-current-sunxi_21.02.2_armhf.deb')
@@ -345,6 +354,7 @@ def test_m68k_mcf5208evb(self):
file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
self.do_test_advcal_2018(file_path, 'sanity-clause.elf')
+@skip("Test currently broken") # Console stuck as of 5.2-rc1
def test_microblaze_s3adsp1800(self):
"""
:avocado: tags=arch:microblaze
@@ -379,6 +389,7 @@ def test_or1k_sim(self):
file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
self.do_test_advcal_2018(file_path, 'vmlinux')
+@skip("nios2 emulation is buggy under record/replay")
def test_nios2_10m50(self):
"""
:avocado: tags=arch:nios2
--
2.39.2
[PULL 21/22] readthodocs: fully specify a build environment
This is now expected by rtd so I've expanded using their example as 22.04 is one of our supported platforms. I tried to work out if there was an easy way to re-generate a requirements.txt from our pythondeps.toml but in the end went for the easier solution. Cc: Signed-off-by: Alex Bennée Message-Id: <20231221174200.2693694-1-alex.ben...@linaro.org> diff --git a/docs/requirements.txt b/docs/requirements.txt new file mode 100644 index 000..691e5218ec7 --- /dev/null +++ b/docs/requirements.txt @@ -0,0 +1,2 @@ +sphinx==5.3.0 +sphinx_rtd_theme==1.1.1 diff --git a/.readthedocs.yml b/.readthedocs.yml index 7fb7b8dd61a..0b262469ce6 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -5,16 +5,21 @@ # Required version: 2 +# Set the version of Python and other tools you might need +build: + os: ubuntu-22.04 + tools: +python: "3.11" + # Build documentation in the docs/ directory with Sphinx sphinx: configuration: docs/conf.py +# We recommend specifying your dependencies to enable reproducible builds: +# https://docs.readthedocs.io/en/stable/guides/reproducible-builds.html +python: + install: +- requirements: docs/requirements.txt + # We want all the document formats formats: all - -# For consistency, we require that QEMU's Sphinx extensions -# run with at least the same minimum version of Python that -# we require for other Python in our codebase (our conf.py -# enforces this, and some code needs it.) -python: - version: 3.6 -- 2.39.2
[PULL 18/22] tests/unit: Bump test-crypto-block test timeout to 5 minutes
From: Thomas Huth
When running the tests in slow mode on a very loaded system and with
--enable-debug, the test-crypto-block can take longer than 4 minutes.
Bump the timeout to 5 minutes to make sure that it also passes in
such situations.
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-15-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index 937e1ebd356..06595321223 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -173,6 +173,7 @@ test_env.set('G_TEST_BUILDDIR', meson.current_build_dir())
slow_tests = {
'test-aio-multithread' : 120,
+ 'test-crypto-block' : 300,
'test-crypto-tlscredsx509': 45,
'test-crypto-tlssession': 45
}
--
2.39.2
[PULL 19/22] tests/fp: Bump fp-test-mulAdd test timeout to 3 minutes
From: Thomas Huth
When running the tests in slow mode with --enable-debug on a very loaded
system, the fp-test-mulAdd test can take longer than 2 minutes. Bump the
timeout to three minutes to make sure it passes in such situations, too.
Signed-off-by: Thomas Huth
Message-Id: <20231215070357.10888-16-th...@redhat.com>
Signed-off-by: Alex Bennée
diff --git a/tests/fp/meson.build b/tests/fp/meson.build
index 4ab89aaa960..114b4b483ea 100644
--- a/tests/fp/meson.build
+++ b/tests/fp/meson.build
@@ -124,7 +124,7 @@ test('fp-test-mulAdd', fptest,
# no fptest_rounding_args
args: fptest_args +
['f16_mulAdd', 'f32_mulAdd', 'f64_mulAdd', 'f128_mulAdd'],
- suite: ['softfloat-slow', 'softfloat-ops-slow', 'slow'], timeout: 90)
+ suite: ['softfloat-slow', 'softfloat-ops-slow', 'slow'], timeout: 180)
executable(
'fp-bench',
--
2.39.2
Re: [PULL 22/22] Revert "tests/avocado: remove skips from replay_kernel"
On Fri, 12 Jan 2024 at 11:11, Alex Bennée wrote:
>
> This reverts commit c2ef5ee89d76f0ab77c4dd6a1c9eeed4d35d20ed.
>
> While the fixes for #2010 and #2013 have improved things locally it
> seems GitLab still continues to be flaky.
>
> Signed-off-by: Alex Bennée
>
> diff --git a/tests/avocado/replay_kernel.py b/tests/avocado/replay_kernel.py
> index 6fdcbd6ac3d..1eaa36444cb 100644
> --- a/tests/avocado/replay_kernel.py
> +++ b/tests/avocado/replay_kernel.py
> @@ -98,10 +98,13 @@ def test_i386_pc(self):
>
> self.run_rr(kernel_path, kernel_command_line, console_pattern,
> shift=5)
>
> +# See https://gitlab.com/qemu-project/qemu/-/issues/2010
> +@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test sometimes gets
> stuck')
> def test_x86_64_pc(self):
> """
> :avocado: tags=arch:x86_64
> :avocado: tags=machine:pc
> +:avocado: tags=flaky
> """
> kernel_url = ('https://archives.fedoraproject.org/pub/archive/fedora'
>
> '/linux/releases/29/Everything/x86_64/os/images/pxeboot'
> @@ -132,6 +135,8 @@ def test_mips_malta(self):
>
> self.run_rr(kernel_path, kernel_command_line, console_pattern,
> shift=5)
>
> +# See https://gitlab.com/qemu-project/qemu/-/issues/2013
> +@skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test is unstable on
> GitLab')
> def test_mips64el_malta(self):
> """
> This test requires the ar tool to extract "data.tar.gz" from
These gitlab issues are both currently closed -- if we think the
problem is still present and are re-introducing the skip lines,
we should re-open the issues, I think.
thanks
-- PMM
Re: [PATCH 1/2] target/ppc/cpu-models: Rename power5+ and power7+ for new QOM naming rules
On 1/12/24 11:55, Thomas Huth wrote:
On 12/01/2024 06.21, Harsh Prateek Bora wrote:
On 1/12/24 10:42, Thomas Huth wrote:
On 12/01/2024 05.57, Harsh Prateek Bora wrote:
On 1/11/24 22:16, Thomas Huth wrote:
The character "+" is now forbidden in QOM device names (see commit
b447378e1217 - "Limit type names to alphanumerical and some few special
characters"). For the "power5+" and "power7+" CPU names, there is
currently a hack in type_name_is_valid() to still allow them for
compatibility reasons. However, there is a much nicer solution for this:
Simply use aliases! This way we can still support the old names without
the need for the ugly hack in type_name_is_valid().
Signed-off-by: Thomas Huth
---
hw/ppc/spapr_cpu_core.c | 4 ++--
qom/object.c | 4
target/ppc/cpu-models.c | 10 ++
3 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 5aa1ed474a..214b7a03d8 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -389,9 +389,9 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
DEFINE_SPAPR_CPU_CORE_TYPE("970_v2.2"),
DEFINE_SPAPR_CPU_CORE_TYPE("970mp_v1.0"),
DEFINE_SPAPR_CPU_CORE_TYPE("970mp_v1.1"),
- DEFINE_SPAPR_CPU_CORE_TYPE("power5+_v2.1"),
+ DEFINE_SPAPR_CPU_CORE_TYPE("power5plus_v2.1"),
DEFINE_SPAPR_CPU_CORE_TYPE("power7_v2.3"),
- DEFINE_SPAPR_CPU_CORE_TYPE("power7+_v2.1"),
+ DEFINE_SPAPR_CPU_CORE_TYPE("power7plus_v2.1"),
Will using Power5x, Power7x be a better naming than using 'plus' suffix ?
The "x" looks like a placeholder to me, so it could be confused with power50,
power51, power52, etc. ...?
But actually, I was thinking about using "power5p" and "power7p" first, so if the whole
"plus" looks too long for you, would "p" be an option instead?
Hmm .. I would certainly vote for 'p' over 'plus'.
Ok, I don't mind either way ... does anybody else have any preferences?
p is fine.
Thanks,
C.
[PATCH v2] load_elf: fix iterators' types for elf file processing
i and size should be the same type as ehdr.e_phnum (Elf32_Half or
Elf64_Half) to avoid overflows. So the bigger one is chosen.
j should be the same type as file_size for the same reasons.
This commit fixes a minor bug, maybe even a typo.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Fixes: 7ef295ea5b ("loader: Add data swap option to load-elf")
Signed-off-by: Anastasia Belova
---
include/hw/elf_ops.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
index 0a5c258fe6..6e807708f3 100644
--- a/include/hw/elf_ops.h
+++ b/include/hw/elf_ops.h
@@ -325,7 +325,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
{
struct elfhdr ehdr;
struct elf_phdr *phdr = NULL, *ph;
-int size, i;
+Elf64_Half size, i;
ssize_t total_size;
elf_word mem_size, file_size, data_offset;
uint64_t addr, low = (uint64_t)-1, high = 0;
@@ -464,7 +464,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
* the ROM overlap check in loader.c, so we don't try to
* explicitly detect those here.
*/
-int j;
+Elf64_Half j;
elf_word zero_start = ph->p_paddr + file_size;
elf_word zero_end = ph->p_paddr + mem_size;
@@ -500,7 +500,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int fd,
}
if (data_swab) {
-int j;
+elf_word j;
for (j = 0; j < file_size; j += (1 << data_swab)) {
uint8_t *dp = data + j;
switch (data_swab) {
--
2.30.2
Re: [PATCH v2] load_elf: fix iterators' types for elf file processing
On Fri, 12 Jan 2024 at 11:46, Anastasia Belova wrote:
>
> i and size should be the same type as ehdr.e_phnum (Elf32_Half or
> Elf64_Half) to avoid overflows. So the bigger one is chosen.
>
> j should be the same type as file_size for the same reasons.
>
> This commit fixes a minor bug, maybe even a typo.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 7ef295ea5b ("loader: Add data swap option to load-elf")
> Signed-off-by: Anastasia Belova
> ---
> include/hw/elf_ops.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/elf_ops.h b/include/hw/elf_ops.h
> index 0a5c258fe6..6e807708f3 100644
> --- a/include/hw/elf_ops.h
> +++ b/include/hw/elf_ops.h
> @@ -325,7 +325,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int
> fd,
> {
> struct elfhdr ehdr;
> struct elf_phdr *phdr = NULL, *ph;
> -int size, i;
> +Elf64_Half size, i;
Elf64_Half is a 16 bit type -- this doesn't seem right.
In particular we use 'size' to store "ehdr.e_phnum * sizeof(phdr[0])"
so even if we know e_phnum fits in a 16 bit type the
multiplication is not guaranteed to; so this change actually
introduces a bug. The type we need for what we're doing needs
to be big enough to store the result of that multiplication,
so anything bigger than 16 bits will be fine, including 'int'
(since we know the RHS is a small number). We could change
this to 'size_t' or 'ssize_t', I suppose, but it doesn't
really seem necessary.
(Side note: whatever your static analyser is could presumbaly be
doing better about identifying overflows here -- it ought to be
able to figure out that "unsigned 16 bit value * constant 64"
is not going to overflow a signed 32 bit type.)
i is only used as the iterator through the program headers,
so it wouldn't be wrong to make it a 16 bit type, but there's
really no need to constrain it -- an 'int' is fine.
Also, these functions have macro magic so we create both the
32-bit and 64-bit elf versions from one bit of source, so it
is not in general right for them to have a type in them that
is specific to one or the other, as Elf64_Half is.
> ssize_t total_size;
> elf_word mem_size, file_size, data_offset;
> uint64_t addr, low = (uint64_t)-1, high = 0;
> @@ -464,7 +464,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int
> fd,
> * the ROM overlap check in loader.c, so we don't try to
> * explicitly detect those here.
> */
> -int j;
> +Elf64_Half j;
This case is like 'i' above -- it's not a bug to use a 16 bit
type for the iterator variable, but it isn't necessary,
and we shouldn't use an Elf64* type.
> elf_word zero_start = ph->p_paddr + file_size;
> elf_word zero_end = ph->p_paddr + mem_size;
>
> @@ -500,7 +500,7 @@ static ssize_t glue(load_elf, SZ)(const char *name, int
> fd,
> }
>
> if (data_swab) {
> -int j;
> +elf_word j;
> for (j = 0; j < file_size; j += (1 << data_swab)) {
> uint8_t *dp = data + j;
> switch (data_swab) {
This change is OK and necessary. It fixes a minor bug:
if we are loading an ELF file that contains a segment
whose data in the ELF file is larger than 2GB and we need
to byteswap that data, we would previously get this wrong.
(We should mention this in the commit message.)
thanks
-- PMM
Re: Re: [PULL 0/6] Firmware/edk2 20231213 patches
On Thu, 11 Jan 2024 at 16:28, Gerd Hoffmann wrote: > > On Thu, Jan 11, 2024 at 04:02:38PM +, Peter Maydell wrote: > > On Thu, 11 Jan 2024 at 15:52, Peter Maydell > > wrote: > > > > > > On Thu, 11 Jan 2024 at 14:01, Gerd Hoffmann wrote: > > > > > > > > Ping. > > > > > > > > As expected this missed the 8.2 boat. Now the devel tree is open again > > > > and people are back from xmas + new year vacations, can this be picked > > > > up for master and eventually 8.2-stable? > > > > > > I can queue it, sure. Do we need to respin it to add cc: qemu-stable > > > tags, or can it be applied as-is ? > > > > ...PS did you mean to cc qemu-stable, not the nonexistent edk2-stable > > on this pullreq email? > > Yes, Cc'ing qemu-stable was the intention, thanks for fixing it up. > > I'd leave it to the stable maintainer(s). If they prefer a respin with > Cc qemu-stable added to all patches I surely can do that. If being > notified with this reply is good enough I'm happy too. Well, it all made it through the CI fine, and I think MJT is fairly flexible about stable- notifications, so I'm going to push this merge to git. Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any user-visible changes. -- PMM
Re: [PULL 00/14] loongarch-to-apply queue
On Thu, 11 Jan 2024 at 11:29, Song Gao wrote: > > The following changes since commit 34eac35f893664eb8545b98142e23d9954722766: > > Merge tag 'pull-riscv-to-apply-20240110' of > https://github.com/alistair23/qemu into staging (2024-01-10 11:41:56 +) > > are available in the Git repository at: > > https://gitlab.com/gaosong/qemu.git tags/pull-loongarch-20240111 > > for you to fetch changes up to 428a6ef4396aa910c86e16c1e4409e3927a3698e: > > hw/intc/loongarch_extioi: Add vmstate post_load support (2024-01-11 > 19:22:47 +0800) > > > pull-loongarch-20240111 > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0 for any user-visible changes. -- PMM
Re: [PATCH 0/2] ppc: Rename power5+ and power7+ for the new QOM naming rules
On Thu, Jan 11, 2024 at 17:46:50 +0100, Thomas Huth wrote: > We can get rid of the "power5+" / "power7+" hack in qom/object.c > by using CPU aliases for those names instead (first patch). > > I think in the long run, we should get rid of the names with a "+" > in it completely, so the second patch suggests to deprecate those, > but I'd also be fine if we keep the aliases around, so in that case > please ignore the second patch. > > Thomas Huth (2): > target/ppc/cpu-models: Rename power5+ and power7+ for new QOM naming > rules > docs/about: Deprecate the old "power5+" and "power7+" CPU names libvirt seems to be explicitly referencing power7+ in the code, so I guess we'll need code to translate the + versions to the spellt-out version to preserve compatibility.
[PATCH 11/88] esp.c: remove unused case from esp_pdma_read()
The do_cmd variable is only set for the MESSAGE OUT and COMMAND phases i.e.
those which involve transfers from the host to the SCSI bus, and so the unused
case can be removed.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 9893840255..6191c17f10 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -187,12 +187,7 @@ static uint8_t esp_pdma_read(ESPState *s)
{
uint8_t val;
-if (s->do_cmd) {
-val = esp_fifo_pop(&s->cmdfifo);
-} else {
-val = esp_fifo_pop(&s->fifo);
-}
-
+val = esp_fifo_pop(&s->fifo);
return val;
}
--
2.39.2
[PATCH 14/88] esp.c: introduce esp_set_phase() helper function
This function is used to set the current SCSI bus phase in the ESP_RSTAT
register
without affecting any of flag bits.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c| 51 ++--
hw/scsi/trace-events | 1 +
2 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f08b816aba..3fc7417d7c 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -183,6 +183,19 @@ static uint32_t esp_get_stc(ESPState *s)
return dmalen;
}
+static const char *esp_phase_names[8] = {
+"DATA OUT", "DATA IN", "COMMAND", "STATUS",
+"(reserved)", "(reserved)", "MESSAGE OUT", "MESSAGE IN"
+};
+
+static void esp_set_phase(ESPState *s, uint8_t phase)
+{
+s->rregs[ESP_RSTAT] &= ~7;
+s->rregs[ESP_RSTAT] |= phase;
+
+trace_esp_set_phase(esp_phase_names[phase]);
+}
+
static uint8_t esp_pdma_read(ESPState *s)
{
uint8_t val;
@@ -316,9 +329,9 @@ static void do_command_phase(ESPState *s)
* complete before raising the command completion interrupt
*/
s->data_in_ready = false;
-s->rregs[ESP_RSTAT] |= STAT_DI;
+esp_set_phase(s, STAT_DI);
} else {
-s->rregs[ESP_RSTAT] |= STAT_DO;
+esp_set_phase(s, STAT_DO);
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
esp_raise_irq(s);
esp_lower_drq(s);
@@ -394,7 +407,7 @@ static void handle_satn(ESPState *s)
s->do_cmd = 1;
/* Target present, but no cmd yet - switch to command phase */
s->rregs[ESP_RSEQ] = SEQ_CD;
-s->rregs[ESP_RSTAT] = STAT_CD;
+esp_set_phase(s, STAT_CD);
}
}
@@ -439,7 +452,7 @@ static void handle_s_without_atn(ESPState *s)
s->do_cmd = 1;
/* Target present, but no cmd yet - switch to command phase */
s->rregs[ESP_RSEQ] = SEQ_CD;
-s->rregs[ESP_RSTAT] = STAT_CD;
+esp_set_phase(s, STAT_CD);
}
}
@@ -457,7 +470,8 @@ static void satn_stop_pdma_cb(ESPState *s)
trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
s->do_cmd = 1;
s->cmdfifo_cdb_offset = 1;
-s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
+esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
s->rregs[ESP_RSEQ] = SEQ_CD;
esp_raise_irq(s);
@@ -481,7 +495,7 @@ static void handle_satn_stop(ESPState *s)
trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
s->do_cmd = 1;
s->cmdfifo_cdb_offset = 1;
-s->rregs[ESP_RSTAT] = STAT_MO;
+esp_set_phase(s, STAT_MO);
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
s->rregs[ESP_RSEQ] = SEQ_MO;
esp_raise_irq(s);
@@ -492,13 +506,14 @@ static void handle_satn_stop(ESPState *s)
s->do_cmd = 1;
/* Target present, switch to message out phase */
s->rregs[ESP_RSEQ] = SEQ_MO;
-s->rregs[ESP_RSTAT] = STAT_MO;
+esp_set_phase(s, STAT_MO);
}
}
static void write_response_pdma_cb(ESPState *s)
{
-s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
+esp_set_phase(s, STAT_ST);
+s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
s->rregs[ESP_RSEQ] = SEQ_CD;
esp_raise_irq(s);
@@ -516,7 +531,8 @@ static void write_response(ESPState *s)
if (s->dma) {
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, buf, 2);
-s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
+esp_set_phase(s, STAT_ST);
+s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
s->rregs[ESP_RSEQ] = SEQ_CD;
} else {
@@ -575,7 +591,8 @@ static void do_dma_pdma_cb(ESPState *s)
* and then switch to command phase
*/
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
-s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
+esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
@@ -681,7 +698,8 @@ static void esp_do_dma(ESPState *s)
* and then switch to command phase
*/
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
-s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
+esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
@@ -810,7 +828,8 @@ static void esp_do_nodma(ESPState *s)
* and then switch to command phase
*/
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
-s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
+esp_set_phase(s, STAT_CD);
+s->rregs[ESP_RSTAT] |= STAT_TC;
[PATCH 05/88] esp: move esp_select() to ESP selection commands from get_cmd()
Since the DREQ value depends upon the result of the selection process, add a
workaround to each esp_select() to manually assert DREQ durring the MESSAGE OUT
and COMMAND phases.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 25 ++---
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 89fce05e58..8c1c6bfc1c 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -263,10 +263,6 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
dmalen = MIN(fifo8_num_free(&s->cmdfifo), dmalen);
fifo8_push_all(&s->cmdfifo, buf, dmalen);
} else {
-if (esp_select(s) < 0) {
-return -1;
-}
-esp_raise_drq(s);
return 0;
}
} else {
@@ -280,9 +276,6 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
}
trace_esp_get_cmd(dmalen, target);
-if (esp_select(s) < 0) {
-return -1;
-}
return dmalen;
}
@@ -380,12 +373,18 @@ static void handle_satn(ESPState *s)
return;
}
esp_set_pdma_cb(s, SATN_PDMA_CB);
+if (esp_select(s) < 0) {
+return;
+}
cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
if (cmdlen > 0) {
s->cmdfifo_cdb_offset = 1;
s->do_cmd = 0;
do_cmd(s);
} else if (cmdlen == 0) {
+if (s->dma) {
+esp_raise_drq(s);
+}
s->do_cmd = 1;
/* Target present, but no cmd yet - switch to command phase */
s->rregs[ESP_RSEQ] = SEQ_CD;
@@ -411,12 +410,18 @@ static void handle_s_without_atn(ESPState *s)
return;
}
esp_set_pdma_cb(s, S_WITHOUT_SATN_PDMA_CB);
+if (esp_select(s) < 0) {
+return;
+}
cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
if (cmdlen > 0) {
s->cmdfifo_cdb_offset = 0;
s->do_cmd = 0;
do_cmd(s);
} else if (cmdlen == 0) {
+if (s->dma) {
+esp_raise_drq(s);
+}
s->do_cmd = 1;
/* Target present, but no cmd yet - switch to command phase */
s->rregs[ESP_RSEQ] = SEQ_CD;
@@ -446,6 +451,9 @@ static void handle_satn_stop(ESPState *s)
return;
}
esp_set_pdma_cb(s, SATN_STOP_PDMA_CB);
+if (esp_select(s) < 0) {
+return;
+}
cmdlen = get_cmd(s, 1);
if (cmdlen > 0) {
trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
@@ -456,6 +464,9 @@ static void handle_satn_stop(ESPState *s)
s->rregs[ESP_RSEQ] = SEQ_MO;
esp_raise_irq(s);
} else if (cmdlen == 0) {
+if (s->dma) {
+esp_raise_drq(s);
+}
s->do_cmd = 1;
/* Target present, switch to message out phase */
s->rregs[ESP_RSEQ] = SEQ_MO;
--
2.39.2
[PATCH 02/88] esp: move existing request cancel check into esp_select()
Since get_cmd() can be called multiple times during a mixed FIFO/DMA request,
move the existing request cancel check into esp_select() which always occurs
at the start of new SCSI request.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 68d07edc05..b382865426 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -209,6 +209,11 @@ static int esp_select(ESPState *s)
s->ti_size = 0;
fifo8_reset(&s->fifo);
+if (s->current_req) {
+/* Started a new command before the old one finished. Cancel it. */
+scsi_req_cancel(s->current_req);
+}
+
s->current_dev = scsi_device_find(&s->bus, 0, target, 0);
if (!s->current_dev) {
/* No such drive */
@@ -235,11 +240,6 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
uint32_t dmalen, n;
int target;
-if (s->current_req) {
-/* Started a new command before the old one finished. Cancel it. */
-scsi_req_cancel(s->current_req);
-}
-
target = s->wregs[ESP_WBUSID] & BUSID_DID;
if (s->dma) {
dmalen = MIN(esp_get_tc(s), maxlen);
--
2.39.2
[PATCH 07/88] esp: start removal of manual STAT_TC setting when transfer counter reaches zero
This should be exclusively managed by esp_set_tc() rather than being manually
set in multiple places. Start by removing the occurrences exclusive to PDMA
and command completion which are those that can be currently removed without
affecting any test images.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index c7b79a2949..e717b2e216 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -611,11 +611,6 @@ static void do_dma_pdma_cb(ESPState *s)
s->async_len -= len;
s->ti_size -= len;
esp_set_tc(s, esp_get_tc(s) - len);
-
-if (esp_get_tc(s) == 0) {
-/* Indicate transfer to FIFO is complete */
- s->rregs[ESP_RSTAT] |= STAT_TC;
-}
return;
}
@@ -720,9 +715,6 @@ static void esp_do_dma(ESPState *s)
esp_set_tc(s, esp_get_tc(s) - len);
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
-
-/* Indicate transfer to FIFO is complete */
-s->rregs[ESP_RSTAT] |= STAT_TC;
return;
}
}
@@ -870,7 +862,8 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
* transfers from the target the last byte is still in the FIFO
*/
if (s->ti_size == 0) {
-s->rregs[ESP_RSTAT] = STAT_TC | STAT_ST;
+s->rregs[ESP_RSTAT] &= ~7;
+s->rregs[ESP_RSTAT] |= STAT_ST;
esp_dma_done(s);
esp_lower_drq(s);
}
--
2.39.2
[PATCH 10/88] esp: move buffer and TC logic into separate to/from device paths in esp_do_dma()
The ultimate aim is to for esp_do_dma() behaviour to be determined by the SCSI
bus phase, in which case it is necessary to have separate to/from device paths.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 65 +++
1 file changed, 40 insertions(+), 25 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 63c828c1b2..9893840255 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -678,14 +678,53 @@ static void esp_do_dma(ESPState *s)
if (to_device) {
if (s->dma_memory_read) {
s->dma_memory_read(s->dma_opaque, s->async_buf, len);
+
+esp_set_tc(s, esp_get_tc(s) - len);
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size += len;
+
+if (s->async_len == 0) {
+scsi_req_continue(s->current_req);
+/*
+ * If there is still data to be read from the device then
+ * complete the DMA operation immediately. Otherwise defer
+ * until the scsi layer has completed.
+ */
+return;
+}
+
+/* Partially filled a scsi buffer. Complete immediately. */
+esp_dma_done(s);
+esp_lower_drq(s);
} else {
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
-return;
}
} else {
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, s->async_buf, len);
+
+esp_set_tc(s, esp_get_tc(s) - len);
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size -= len;
+
+if (s->async_len == 0) {
+scsi_req_continue(s->current_req);
+/*
+ * If there is still data to be read from the device then
+ * complete the DMA operation immediately. Otherwise defer
+ * until the scsi layer has completed.
+ */
+if (esp_get_tc(s) != 0 || s->ti_size == 0) {
+return;
+}
+}
+
+/* Partially filled a scsi buffer. Complete immediately. */
+esp_dma_done(s);
+esp_lower_drq(s);
} else {
/* Adjust TC for any leftover data in the FIFO */
if (!fifo8_is_empty(&s->fifo)) {
@@ -713,32 +752,8 @@ static void esp_do_dma(ESPState *s)
esp_set_tc(s, esp_get_tc(s) - len);
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
-return;
}
}
-esp_set_tc(s, esp_get_tc(s) - len);
-s->async_buf += len;
-s->async_len -= len;
-if (to_device) {
-s->ti_size += len;
-} else {
-s->ti_size -= len;
-}
-if (s->async_len == 0) {
-scsi_req_continue(s->current_req);
-/*
- * If there is still data to be read from the device then
- * complete the DMA operation immediately. Otherwise defer
- * until the scsi layer has completed.
- */
-if (to_device || esp_get_tc(s) != 0 || s->ti_size == 0) {
-return;
-}
-}
-
-/* Partially filled a scsi buffer. Complete immediately. */
-esp_dma_done(s);
-esp_lower_drq(s);
}
static void esp_do_nodma(ESPState *s)
--
2.39.2
[PATCH 21/88] esp.c: update condition for esp_dma_done() in esp_do_dma() to device path
Ensure that esp_dma_done() is only called when TC is zero, which is currently
always the case for DMA transfers.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 96723efcf3..06be9f2e74 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -725,9 +725,11 @@ static void esp_do_dma(ESPState *s)
return;
}
-/* Partially filled a scsi buffer. Complete immediately. */
-esp_dma_done(s);
-esp_lower_drq(s);
+if (esp_get_tc(s) == 0) {
+/* Partially filled a scsi buffer. Complete immediately. */
+esp_dma_done(s);
+esp_lower_drq(s);
+}
} else {
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
--
2.39.2
[PATCH 06/88] esp: update esp_set_tc() to set STAT_TC flag
This flag is set once the transfer counter counts down to zero.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 8c1c6bfc1c..c7b79a2949 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -161,9 +161,15 @@ static uint32_t esp_get_tc(ESPState *s)
static void esp_set_tc(ESPState *s, uint32_t dmalen)
{
+uint32_t old_tc = esp_get_tc(s);
+
s->rregs[ESP_TCLO] = dmalen;
s->rregs[ESP_TCMID] = dmalen >> 8;
s->rregs[ESP_TCHI] = dmalen >> 16;
+
+if (old_tc && dmalen == 0) {
+s->rregs[ESP_RSTAT] |= STAT_TC;
+}
}
static uint32_t esp_get_stc(ESPState *s)
--
2.39.2
[PATCH 28/88] esp.c: consolidate async_len and TC == 0 checks in do_dma_pdma_cb() and esp_do_dma()
Ensure that the async_len checks for requesting data from the SCSI layer and
the TC == 0 checks to detect the end of the DMA transfer are consistent in both
do_dma_pdma_cb() and esp_do_dma(). In particular this involves adding the check
to see if the FIFO is at its low threshold since PDMA and mixed DMA and non-DMA
requests can leave data remaining in the FIFO.
At the same time update all the comments so that they are also consistent
between
all similar code paths.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 44 +++-
1 file changed, 31 insertions(+), 13 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 6b0811d3ce..f20026c3dc 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -604,12 +604,13 @@ static void do_dma_pdma_cb(ESPState *s)
s->async_len -= n;
s->ti_size += n;
-if (s->async_len == 0) {
+if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
+/* Defer until the scsi layer has completed */
scsi_req_continue(s->current_req);
return;
}
-if (esp_get_tc(s) == 0) {
+if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
esp_lower_drq(s);
esp_dma_done(s);
}
@@ -706,24 +707,30 @@ static void esp_do_dma(ESPState *s)
s->async_len -= len;
s->ti_size += len;
-if (s->async_len == 0) {
+if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
+/* Defer until the scsi layer has completed */
scsi_req_continue(s->current_req);
-/*
- * If there is still data to be read from the device then
- * complete the DMA operation immediately. Otherwise defer
- * until the scsi layer has completed.
- */
return;
}
-if (esp_get_tc(s) == 0) {
-/* Partially filled a scsi buffer. Complete immediately. */
+if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
esp_dma_done(s);
esp_lower_drq(s);
}
} else {
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
+
+if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
+/* Defer until the scsi layer has completed */
+scsi_req_continue(s->current_req);
+return;
+}
+
+if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
+esp_dma_done(s);
+esp_lower_drq(s);
+}
}
} else {
if (s->dma_memory_write) {
@@ -734,13 +741,13 @@ static void esp_do_dma(ESPState *s)
s->async_len -= len;
s->ti_size -= len;
-if (s->async_len == 0) {
+if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
+/* Defer until the scsi layer has completed */
scsi_req_continue(s->current_req);
return;
}
-if (esp_get_tc(s) == 0) {
-/* Partially filled a scsi buffer. Complete immediately. */
+if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
esp_dma_done(s);
esp_lower_drq(s);
}
@@ -754,6 +761,17 @@ static void esp_do_dma(ESPState *s)
esp_set_tc(s, esp_get_tc(s) - len);
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
+
+if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
+/* Defer until the scsi layer has completed */
+scsi_req_continue(s->current_req);
+return;
+}
+
+if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
+esp_lower_drq(s);
+esp_dma_done(s);
+}
}
}
}
--
2.39.2
[PATCH 35/88] esp.c: move end of SCSI transfer check after TC adjustment in do_dma_pdma_cb()
Now it is possible to move the end of SCSI transfer check to after the TC
adjustment in do_dma_pdma_cb() when transferring data from the device
without triggering an assert() in the SCSI code. This brings this check in
line with all the others in esp_do_dma() and do_dma_pdma_cb().
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 9647be4cc3..df4d5f8811 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -615,15 +615,6 @@ static void do_dma_pdma_cb(ESPState *s)
esp_dma_ti_check(s);
} else {
-if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
-/* Defer until the scsi layer has completed */
-scsi_req_continue(s->current_req);
-s->data_in_ready = false;
-return;
-}
-
-esp_dma_ti_check(s);
-
/* Copy device data to FIFO */
len = MIN(s->async_len, esp_get_tc(s));
len = MIN(len, fifo8_num_free(&s->fifo));
@@ -632,6 +623,15 @@ static void do_dma_pdma_cb(ESPState *s)
s->async_len -= len;
s->ti_size -= len;
esp_set_tc(s, esp_get_tc(s) - len);
+
+if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
+/* Defer until the scsi layer has completed */
+scsi_req_continue(s->current_req);
+s->data_in_ready = false;
+return;
+}
+
+esp_dma_ti_check(s);
}
}
--
2.39.2
[PATCH 13/88] esp.c: decrement the TC during MESSAGE OUT and COMMAND phases
This is to ensure that STAT_TC is triggered during the right parts of the
transfer when it is controlled exclusively by the TC.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 9e9bbe8431..f08b816aba 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -259,6 +259,7 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
s->dma_memory_read(s->dma_opaque, buf, dmalen);
dmalen = MIN(fifo8_num_free(&s->cmdfifo), dmalen);
fifo8_push_all(&s->cmdfifo, buf, dmalen);
+esp_set_tc(s, esp_get_tc(s) - dmalen);
} else {
return 0;
}
@@ -657,6 +658,7 @@ static void esp_do_dma(ESPState *s)
len = MIN(len, fifo8_num_free(&s->cmdfifo));
s->dma_memory_read(s->dma_opaque, buf, len);
fifo8_push_all(&s->cmdfifo, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
} else {
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
--
2.39.2
[PATCH 26/88] esp.c: remove unneeded if() check in esp_transfer_data()
The following ti_cmd checks ensure that only DMA and non-DMA TI commmands will
can call into the esp_do_dma() and esp_do_nodma() callbacks.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 17 +++--
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 3db90c9ab7..96123c5f7d 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -916,16 +916,13 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
esp_raise_irq(s);
}
-if (s->ti_cmd == 0) {
-/*
- * Always perform the initial transfer upon reception of the next TI
- * command to ensure the DMA/non-DMA status of the command is correct.
- * It is not possible to use s->dma directly in the section below as
- * some OSs send non-DMA NOP commands after a DMA transfer. Hence if
the
- * async data transfer is delayed then s->dma is set incorrectly.
- */
-return;
-}
+/*
+ * Always perform the initial transfer upon reception of the next TI
+ * command to ensure the DMA/non-DMA status of the command is correct.
+ * It is not possible to use s->dma directly in the section below as
+ * some OSs send non-DMA NOP commands after a DMA transfer. Hence if the
+ * async data transfer is delayed then s->dma is set incorrectly.
+ */
if (s->ti_cmd == (CMD_TI | CMD_DMA)) {
if (dmalen) {
--
2.39.2
[PATCH 29/88] esp.c: fix premature end of phase logic esp_command_complete
There are two cases here: the first is when the TI command underflows, in which
case we raise INTR_BS to indicate an early change of phase, and the second is
when the TI command overflows because the host requested a transfer for more
data than is available. In the latter case force TC to zero so that the TI
completion logic executes correctly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 22 +-
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f20026c3dc..c6151d306e 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -887,7 +887,6 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
if (s->ti_size != 0) {
trace_esp_command_complete_unexpected();
}
-s->ti_size = 0;
}
s->async_len = 0;
@@ -897,13 +896,26 @@ void esp_command_complete(SCSIRequest *req, size_t resid)
s->status = req->status;
/*
- * If the transfer is finished, switch to status phase. For non-DMA
- * transfers from the target the last byte is still in the FIFO
+ * Switch to status phase. For non-DMA transfers from the target the last
+ * byte is still in the FIFO
*/
+esp_set_phase(s, STAT_ST);
if (s->ti_size == 0) {
-esp_set_phase(s, STAT_ST);
+/*
+ * Transfer complete: force TC to zero just in case a TI command was
+ * requested for more data than the command returns (Solaris 8 does
+ * this)
+ */
+esp_set_tc(s, 0);
esp_dma_done(s);
-esp_lower_drq(s);
+} else {
+/*
+ * Transfer truncated: raise INTR_BS to indicate early change of
+ * phase
+ */
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+s->ti_size = 0;
}
if (s->current_req) {
--
2.39.2
[PATCH 24/88] esp.c: remove TC adjustment in esp_do_dma() from device path
Now that the TC is updated for each PDMA access (rather than once the FIFO is
full) there is no need to adjust the TC at start of each DMA transfer if the
FIFO is not empty.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 5 -
1 file changed, 5 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 1f9902aec0..ec82097a01 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -754,11 +754,6 @@ static void esp_do_dma(ESPState *s)
esp_lower_drq(s);
}
} else {
-/* Adjust TC for any leftover data in the FIFO */
-if (!fifo8_is_empty(&s->fifo)) {
-esp_set_tc(s, esp_get_tc(s) - fifo8_num_used(&s->fifo));
-}
-
/* Copy device data to FIFO */
len = MIN(len, fifo8_num_free(&s->fifo));
fifo8_push_all(&s->fifo, s->async_buf, len);
--
2.39.2
[PATCH 12/88] esp.c: don't accumulate directly into cmdfifo
Instead accumulate in the real FIFO as done in real hardware, and then transfer
to cmdfifo when we're ready to process the MESSAGE OUT and COMMAND phase data.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 49 ++---
1 file changed, 42 insertions(+), 7 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 6191c17f10..9e9bbe8431 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -199,11 +199,7 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
return;
}
-if (s->do_cmd) {
-esp_fifo_push(&s->cmdfifo, val);
-} else {
-esp_fifo_push(&s->fifo, val);
-}
+esp_fifo_push(&s->fifo, val);
dmalen--;
esp_set_tc(s, dmalen);
@@ -358,6 +354,14 @@ static void do_cmd(ESPState *s)
static void satn_pdma_cb(ESPState *s)
{
+uint8_t buf[ESP_FIFO_SZ];
+int n;
+
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
s->cmdfifo_cdb_offset = 1;
s->do_cmd = 0;
@@ -395,6 +399,14 @@ static void handle_satn(ESPState *s)
static void s_without_satn_pdma_cb(ESPState *s)
{
+uint8_t buf[ESP_FIFO_SZ];
+int n;
+
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
s->cmdfifo_cdb_offset = 0;
s->do_cmd = 0;
@@ -432,6 +444,14 @@ static void handle_s_without_atn(ESPState *s)
static void satn_stop_pdma_cb(ESPState *s)
{
+uint8_t buf[ESP_FIFO_SZ];
+int n;
+
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
s->do_cmd = 1;
@@ -523,10 +543,16 @@ static void esp_dma_done(ESPState *s)
static void do_dma_pdma_cb(ESPState *s)
{
int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
+uint8_t buf[ESP_CMDFIFO_SZ];
int len;
uint32_t n;
if (s->do_cmd) {
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
/* Ensure we have received complete command after SATN and stop */
if (esp_get_tc(s) || fifo8_is_empty(&s->cmdfifo)) {
return;
@@ -754,10 +780,16 @@ static void esp_do_dma(ESPState *s)
static void esp_do_nodma(ESPState *s)
{
int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
+uint8_t buf[ESP_FIFO_SZ];
uint32_t cmdlen;
-int len;
+int len, n;
if (s->do_cmd) {
+/* Copy FIFO into cmdfifo */
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
cmdlen = fifo8_num_used(&s->cmdfifo);
trace_esp_handle_ti_cmd(cmdlen);
s->ti_size = 0;
@@ -1159,7 +1191,10 @@ void esp_reg_write(ESPState *s, uint32_t saddr, uint64_t
val)
break;
case ESP_FIFO:
if (s->do_cmd) {
-esp_fifo_push(&s->cmdfifo, val);
+if (!fifo8_is_full(&s->fifo)) {
+esp_fifo_push(&s->fifo, val);
+esp_fifo_push(&s->cmdfifo, fifo8_pop(&s->fifo));
+}
/*
* If any unexpected message out/command phase data is
--
2.39.2
[PATCH 23/88] esp.c: don't immediately raise INTR_BS if SCSI data needed in esp_do_dma()
In the case when more data is requested from the SCSI layer during a DMA data
transfer from a device, don't immediately fall through to the TC check logic.
Otherwise when TC is zero INTR_BS will be raised immediately rather than when
the next set of SCSI data is ready.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index d80a38daa0..1f9902aec0 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -745,6 +745,7 @@ static void esp_do_dma(ESPState *s)
if (s->async_len == 0) {
scsi_req_continue(s->current_req);
+return;
}
if (esp_get_tc(s) == 0) {
--
2.39.2
[PATCH 40/88] esp.c: convert esp_do_nodma() to switch statement based upon SCSI phase
Currently only the DATA IN and DATA OUT phases are supported.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 54 +--
1 file changed, 35 insertions(+), 19 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f6d05b0de7..c1b44e5f18 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -786,7 +786,6 @@ static void esp_do_dma(ESPState *s)
static void esp_do_nodma(ESPState *s)
{
-int to_device = (esp_get_phase(s) == STAT_DO);
uint8_t buf[ESP_FIFO_SZ];
uint32_t cmdlen;
int len, n;
@@ -823,38 +822,55 @@ static void esp_do_nodma(ESPState *s)
return;
}
-if (!s->current_req) {
-return;
-}
-
-if (s->async_len == 0) {
-/* Defer until data is available. */
-return;
-}
-
-if (to_device) {
+switch (esp_get_phase(s)) {
+case STAT_DO:
+if (!s->current_req) {
+return;
+}
+if (s->async_len == 0) {
+/* Defer until data is available. */
+return;
+}
len = MIN(s->async_len, ESP_FIFO_SZ);
len = MIN(len, fifo8_num_used(&s->fifo));
esp_fifo_pop_buf(&s->fifo, s->async_buf, len);
s->async_buf += len;
s->async_len -= len;
s->ti_size += len;
-} else {
+
+if (s->async_len == 0) {
+scsi_req_continue(s->current_req);
+return;
+}
+
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+break;
+
+case STAT_DI:
+if (!s->current_req) {
+return;
+}
+if (s->async_len == 0) {
+/* Defer until data is available. */
+return;
+}
if (fifo8_is_empty(&s->fifo)) {
fifo8_push(&s->fifo, s->async_buf[0]);
s->async_buf++;
s->async_len--;
s->ti_size--;
}
-}
-if (s->async_len == 0) {
-scsi_req_continue(s->current_req);
-return;
-}
+if (s->async_len == 0) {
+scsi_req_continue(s->current_req);
+return;
+}
-s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+break;
+}
}
static void esp_pdma_cb(ESPState *s)
--
2.39.2
[PATCH 09/88] esp: update TC check logic in do_dma_pdma_cb() to check for TC == 0
Invert the logic so that the end of DMA transfer check becomes one that checks
for TC == 0 in the from device path in do_dma_pdma_cb().
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index fecfef7c89..63c828c1b2 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -602,21 +602,19 @@ static void do_dma_pdma_cb(ESPState *s)
return;
}
-if (esp_get_tc(s) != 0) {
-/* Copy device data to FIFO */
-len = MIN(s->async_len, esp_get_tc(s));
-len = MIN(len, fifo8_num_free(&s->fifo));
-fifo8_push_all(&s->fifo, s->async_buf, len);
-s->async_buf += len;
-s->async_len -= len;
-s->ti_size -= len;
-esp_set_tc(s, esp_get_tc(s) - len);
-return;
+if (esp_get_tc(s) == 0) {
+esp_lower_drq(s);
+esp_dma_done(s);
}
-/* Partially filled a scsi buffer. Complete immediately. */
-esp_lower_drq(s);
-esp_dma_done(s);
+/* Copy device data to FIFO */
+len = MIN(s->async_len, esp_get_tc(s));
+len = MIN(len, fifo8_num_free(&s->fifo));
+fifo8_push_all(&s->fifo, s->async_buf, len);
+s->async_buf += len;
+s->async_len -= len;
+s->ti_size -= len;
+esp_set_tc(s, esp_get_tc(s) - len);
}
}
--
2.39.2
[PATCH 30/88] esp.c: move TC and FIFO check logic into esp_dma_done()
This helps simplify the existing implementation.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 39 +++
1 file changed, 11 insertions(+), 28 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index c6151d306e..4d58a49c73 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -545,8 +545,11 @@ static void write_response(ESPState *s)
static void esp_dma_done(ESPState *s)
{
-s->rregs[ESP_RINTR] |= INTR_BS;
-esp_raise_irq(s);
+if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
+s->rregs[ESP_RINTR] |= INTR_BS;
+esp_raise_irq(s);
+esp_lower_drq(s);
+}
}
static void do_dma_pdma_cb(ESPState *s)
@@ -610,12 +613,7 @@ static void do_dma_pdma_cb(ESPState *s)
return;
}
-if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
-esp_lower_drq(s);
-esp_dma_done(s);
-}
-
-return;
+esp_dma_done(s);
} else {
if (s->async_len == 0 && fifo8_num_used(&s->fifo) < 2) {
/* Defer until the scsi layer has completed */
@@ -624,10 +622,7 @@ static void do_dma_pdma_cb(ESPState *s)
return;
}
-if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
-esp_lower_drq(s);
-esp_dma_done(s);
-}
+esp_dma_done(s);
/* Copy device data to FIFO */
len = MIN(s->async_len, esp_get_tc(s));
@@ -713,10 +708,7 @@ static void esp_do_dma(ESPState *s)
return;
}
-if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
-esp_dma_done(s);
-esp_lower_drq(s);
-}
+esp_dma_done(s);
} else {
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
@@ -727,10 +719,7 @@ static void esp_do_dma(ESPState *s)
return;
}
-if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
-esp_dma_done(s);
-esp_lower_drq(s);
-}
+esp_dma_done(s);
}
} else {
if (s->dma_memory_write) {
@@ -747,10 +736,7 @@ static void esp_do_dma(ESPState *s)
return;
}
-if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
-esp_dma_done(s);
-esp_lower_drq(s);
-}
+esp_dma_done(s);
} else {
/* Copy device data to FIFO */
len = MIN(len, fifo8_num_free(&s->fifo));
@@ -768,10 +754,7 @@ static void esp_do_dma(ESPState *s)
return;
}
-if (esp_get_tc(s) == 0 && fifo8_num_used(&s->fifo) < 2) {
-esp_lower_drq(s);
-esp_dma_done(s);
-}
+esp_dma_done(s);
}
}
}
--
2.39.2
[PATCH 36/88] esp.c: remove s_without_satn_pdma_cb() PDMA callback
This can now be handled by the existing do_dma_pdma_cb() function.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 22 +-
include/hw/scsi/esp.h | 1 -
2 files changed, 1 insertion(+), 22 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index df4d5f8811..16cb6c72fd 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -408,23 +408,6 @@ static void handle_satn(ESPState *s)
}
}
-static void s_without_satn_pdma_cb(ESPState *s)
-{
-uint8_t buf[ESP_FIFO_SZ];
-int n;
-
-/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-
-if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
-s->cmdfifo_cdb_offset = 0;
-s->do_cmd = 0;
-do_cmd(s);
-}
-}
-
static void handle_s_without_atn(ESPState *s)
{
int32_t cmdlen;
@@ -433,7 +416,7 @@ static void handle_s_without_atn(ESPState *s)
s->dma_cb = handle_s_without_atn;
return;
}
-esp_set_pdma_cb(s, S_WITHOUT_SATN_PDMA_CB);
+esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
if (esp_select(s) < 0) {
return;
}
@@ -856,9 +839,6 @@ static void esp_pdma_cb(ESPState *s)
case SATN_PDMA_CB:
satn_pdma_cb(s);
break;
-case S_WITHOUT_SATN_PDMA_CB:
-s_without_satn_pdma_cb(s);
-break;
case SATN_STOP_PDMA_CB:
satn_stop_pdma_cb(s);
break;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index 13b17496f8..b727181da9 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -153,7 +153,6 @@ struct SysBusESPState {
/* PDMA callbacks */
enum pdma_cb {
SATN_PDMA_CB = 0,
-S_WITHOUT_SATN_PDMA_CB = 1,
SATN_STOP_PDMA_CB = 2,
WRITE_RESPONSE_PDMA_CB = 3,
DO_DMA_PDMA_CB = 4
--
2.39.2
[PATCH 15/88] esp.c: remove another set of manual STAT_TC updates
Following on from the recent changes to when the TC is updated, it is now
possible to remove another set of manual STAT_TC updates so that its state
is now managed within esp_set_tc().
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 3fc7417d7c..6fd5c8767a 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -513,7 +513,6 @@ static void handle_satn_stop(ESPState *s)
static void write_response_pdma_cb(ESPState *s)
{
esp_set_phase(s, STAT_ST);
-s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
s->rregs[ESP_RSEQ] = SEQ_CD;
esp_raise_irq(s);
@@ -532,7 +531,6 @@ static void write_response(ESPState *s)
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, buf, 2);
esp_set_phase(s, STAT_ST);
-s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
s->rregs[ESP_RSEQ] = SEQ_CD;
} else {
@@ -550,10 +548,8 @@ static void write_response(ESPState *s)
static void esp_dma_done(ESPState *s)
{
-s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RINTR] |= INTR_BS;
s->rregs[ESP_RFLAGS] = 0;
-esp_set_tc(s, 0);
esp_raise_irq(s);
}
@@ -592,7 +588,6 @@ static void do_dma_pdma_cb(ESPState *s)
*/
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
esp_set_phase(s, STAT_CD);
-s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
@@ -699,7 +694,6 @@ static void esp_do_dma(ESPState *s)
*/
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
esp_set_phase(s, STAT_CD);
-s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
@@ -829,7 +823,6 @@ static void esp_do_nodma(ESPState *s)
*/
s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
esp_set_phase(s, STAT_CD);
-s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
@@ -952,7 +945,6 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
* completion interrupt
*/
s->data_in_ready = true;
-s->rregs[ESP_RSTAT] |= STAT_TC;
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
}
@@ -997,7 +989,6 @@ static void handle_ti(ESPState *s)
if (s->dma) {
dmalen = esp_get_tc(s);
trace_esp_handle_ti(dmalen);
-s->rregs[ESP_RSTAT] &= ~STAT_TC;
esp_do_dma(s);
} else {
trace_esp_handle_ti(s->ti_size);
@@ -1152,7 +1143,6 @@ uint64_t esp_reg_read(ESPState *s, uint32_t saddr)
* of the FIFO so switch to status phase
*/
esp_set_phase(s, STAT_ST);
-s->rregs[ESP_RSTAT] |= STAT_TC;
}
}
s->rregs[ESP_FIFO] = esp_fifo_pop(&s->fifo);
--
2.39.2
[PATCH 34/88] esp.c: update esp_do_dma() bypass if async_len is zero to include non-zero transfer check
In the PDMA case the last transfer from the device to the FIFO has occurred
(async_len is zero) but esp_do_dma() is still being called to drain the
remaining FIFO contents.
The additional non-zero transfer check ensures that we still defer the SCSI
layer in the case where we are waiting for data for a TI command or a DMA
enable signal.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 14284ef54a..9647be4cc3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -696,7 +696,7 @@ static void esp_do_dma(ESPState *s)
if (!s->current_req) {
return;
}
-if (s->async_len == 0) {
+if (s->async_len == 0 && esp_get_tc(s) && s->ti_size) {
/* Defer until data is available. */
return;
}
--
2.39.2
[PATCH 17/88] esp.c: don't reset the TC and ESP_RSEQ state when executing a SCSI command
There is no need to manually reset these values as the ESP emulation now
correctly handles them within its existing logic.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f41b2421f9..a4a1f41a40 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -319,10 +319,7 @@ static void do_command_phase(ESPState *s)
s->ti_size = datalen;
fifo8_reset(&s->cmdfifo);
if (datalen != 0) {
-s->rregs[ESP_RSTAT] = STAT_TC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
s->ti_cmd = 0;
-esp_set_tc(s, 0);
if (datalen > 0) {
/*
* Switch to DATA IN phase but wait until initial data xfer is
--
2.39.2
[PATCH 54/88] esp.c: move CMD_ICCS command logic to esp_do_dma()
The special logic in write_response_pdma_cb() is now no longer required since
esp_do_dma() can be used as a direct replacement.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 82 ++-
include/hw/scsi/esp.h | 1 -
2 files changed, 57 insertions(+), 26 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index f69b2709fc..c6e5ddd537 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -446,40 +446,23 @@ static void handle_satn_stop(ESPState *s)
}
}
-static void write_response_pdma_cb(ESPState *s)
-{
-esp_set_phase(s, STAT_ST);
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
-esp_raise_irq(s);
-}
-
static void write_response(ESPState *s)
{
uint8_t buf[2];
trace_esp_write_response(s->status);
-buf[0] = s->status;
-buf[1] = 0;
-
if (s->dma) {
-if (s->dma_memory_write) {
-s->dma_memory_write(s->dma_opaque, buf, 2);
-esp_set_phase(s, STAT_ST);
-s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
-s->rregs[ESP_RSEQ] = SEQ_CD;
-} else {
-esp_set_pdma_cb(s, WRITE_RESPONSE_PDMA_CB);
-esp_raise_drq(s);
-return;
-}
+esp_do_dma(s);
} else {
+buf[0] = s->status;
+buf[1] = 0;
+
fifo8_reset(&s->fifo);
fifo8_push_all(&s->fifo, buf, 2);
s->rregs[ESP_RFLAGS] = 2;
+esp_raise_irq(s);
}
-esp_raise_irq(s);
}
static void esp_dma_ti_check(ESPState *s)
@@ -673,6 +656,58 @@ static void esp_do_dma(ESPState *s)
esp_dma_ti_check(s);
}
break;
+
+case STAT_ST:
+switch (s->rregs[ESP_CMD]) {
+case CMD_ICCS | CMD_DMA:
+len = MIN(len, 1);
+
+if (len) {
+buf[0] = s->status;
+
+if (s->dma_memory_write) {
+s->dma_memory_write(s->dma_opaque, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+} else {
+fifo8_push_all(&s->fifo, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+}
+
+esp_set_phase(s, STAT_MI);
+
+if (esp_get_tc(s) > 0) {
+/* Process any message in phase data */
+esp_do_dma(s);
+}
+}
+break;
+}
+break;
+
+case STAT_MI:
+switch (s->rregs[ESP_CMD]) {
+case CMD_ICCS | CMD_DMA:
+len = MIN(len, 1);
+
+if (len) {
+buf[0] = 0;
+
+if (s->dma_memory_write) {
+s->dma_memory_write(s->dma_opaque, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+} else {
+fifo8_push_all(&s->fifo, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+}
+
+/* Raise end of command interrupt */
+s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
+s->rregs[ESP_RSEQ] = SEQ_CD;
+esp_raise_irq(s);
+}
+break;
+}
+break;
}
}
@@ -773,9 +808,6 @@ static void esp_do_nodma(ESPState *s)
static void esp_pdma_cb(ESPState *s)
{
switch (s->pdma_cb) {
-case WRITE_RESPONSE_PDMA_CB:
-write_response_pdma_cb(s);
-break;
case DO_DMA_PDMA_CB:
esp_do_dma(s);
break;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index a4b2ed115c..0207fdd7a6 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -152,7 +152,6 @@ struct SysBusESPState {
/* PDMA callbacks */
enum pdma_cb {
-WRITE_RESPONSE_PDMA_CB = 3,
DO_DMA_PDMA_CB = 4
};
--
2.39.2
[PATCH 16/88] esp.c: remove MacOS TI workaround that pads FIFO transfers to ESP_FIFO_SZ
This workaround is no longer required with the current code and so can be
removed.
[Note: whilst MacOS itself can boot correctly, removing this hack prevents
a bootable EMILE CDROM from working. This is caused by a separate bug which
will be fixed by a subsequent patch]
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 12
1 file changed, 12 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 6fd5c8767a..f41b2421f9 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -772,18 +772,6 @@ static void esp_do_dma(ESPState *s)
s->async_buf += len;
s->async_len -= len;
s->ti_size -= len;
-
-/*
- * MacOS toolbox uses a TI length of 16 bytes for all commands, so
- * commands shorter than this must be padded accordingly
- */
-if (len < esp_get_tc(s) && esp_get_tc(s) <= ESP_FIFO_SZ) {
-while (fifo8_num_used(&s->fifo) < ESP_FIFO_SZ) {
-esp_fifo_push(&s->fifo, 0);
-len++;
-}
-}
-
esp_set_tc(s, esp_get_tc(s) - len);
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
--
2.39.2
[PATCH 38/88] esp.c: convert esp_do_dma() to switch statement based upon SCSI phase
Currently only the DATA IN and DATA OUT phases are supported.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 40 +++-
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index de8d98082a..67d1d39db2 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -626,7 +626,6 @@ static void do_dma_pdma_cb(ESPState *s)
static void esp_do_dma(ESPState *s)
{
uint32_t len, cmdlen;
-int to_device = (esp_get_phase(s) == STAT_DO);
uint8_t buf[ESP_CMDFIFO_SZ];
int n;
@@ -681,17 +680,19 @@ static void esp_do_dma(ESPState *s)
}
return;
}
-if (!s->current_req) {
-return;
-}
-if (s->async_len == 0 && esp_get_tc(s) && s->ti_size) {
-/* Defer until data is available. */
-return;
-}
-if (len > s->async_len) {
-len = s->async_len;
-}
-if (to_device) {
+
+switch (esp_get_phase(s)) {
+case STAT_DO:
+if (!s->current_req) {
+return;
+}
+if (s->async_len == 0 && esp_get_tc(s) && s->ti_size) {
+/* Defer until data is available. */
+return;
+}
+if (len > s->async_len) {
+len = s->async_len;
+}
if (s->dma_memory_read) {
s->dma_memory_read(s->dma_opaque, s->async_buf, len);
@@ -727,7 +728,19 @@ static void esp_do_dma(ESPState *s)
esp_dma_ti_check(s);
}
-} else {
+break;
+
+case STAT_DI:
+if (!s->current_req) {
+return;
+}
+if (s->async_len == 0 && esp_get_tc(s) && s->ti_size) {
+/* Defer until data is available. */
+return;
+}
+if (len > s->async_len) {
+len = s->async_len;
+}
if (s->dma_memory_write) {
s->dma_memory_write(s->dma_opaque, s->async_buf, len);
@@ -762,6 +775,7 @@ static void esp_do_dma(ESPState *s)
esp_dma_ti_check(s);
}
+break;
}
}
--
2.39.2
[PATCH 47/88] esp.c: untangle MESSAGE OUT and COMMAND phase logic in do_dma_pdma_cb()
This makes it clearer that ATN is asserted until the end of the next TI command
in the MESSAGE OUT phase.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 66 ---
1 file changed, 42 insertions(+), 24 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 2f54fd1285..7ab195f884 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -536,36 +536,30 @@ static void do_dma_pdma_cb(ESPState *s)
{
uint8_t buf[ESP_CMDFIFO_SZ];
int len;
-uint32_t n;
+uint32_t n, cmdlen;
+
+len = esp_get_tc(s);
switch (esp_get_phase(s)) {
case STAT_MO:
-case STAT_CD:
-/* Copy FIFO into cmdfifo */
-n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
-n = MIN(fifo8_num_free(&s->cmdfifo), n);
-fifo8_push_all(&s->cmdfifo, buf, n);
-
-/* Ensure we have received complete command after SATN and stop */
-if (esp_get_tc(s) || fifo8_is_empty(&s->cmdfifo)) {
-return;
+if (s->dma_memory_read) {
+len = MIN(len, fifo8_num_free(&s->cmdfifo));
+s->dma_memory_read(s->dma_opaque, buf, len);
+fifo8_push_all(&s->cmdfifo, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+s->cmdfifo_cdb_offset += len;
+} else {
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+s->cmdfifo_cdb_offset += n;
}
-s->ti_size = 0;
-if (esp_get_phase(s) == STAT_CD) {
-/* No command received */
-if (s->cmdfifo_cdb_offset == fifo8_num_used(&s->cmdfifo)) {
-return;
-}
+esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
+esp_raise_drq(s);
-/* Command has been received */
-do_cmd(s);
-} else {
-/*
- * Extra message out bytes received: update cmdfifo_cdb_offset
- * and then switch to command phase
- */
-s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
+/* ATN remains asserted until TC == 0 */
+if (esp_get_tc(s) == 0) {
esp_set_phase(s, STAT_CD);
s->rregs[ESP_RSEQ] = SEQ_CD;
s->rregs[ESP_RINTR] |= INTR_BS;
@@ -573,6 +567,30 @@ static void do_dma_pdma_cb(ESPState *s)
}
break;
+case STAT_CD:
+cmdlen = fifo8_num_used(&s->cmdfifo);
+trace_esp_do_dma(cmdlen, len);
+if (s->dma_memory_read) {
+len = MIN(len, fifo8_num_free(&s->cmdfifo));
+s->dma_memory_read(s->dma_opaque, buf, len);
+fifo8_push_all(&s->cmdfifo, buf, len);
+esp_set_tc(s, esp_get_tc(s) - len);
+} else {
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+
+esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
+esp_raise_drq(s);
+}
+trace_esp_handle_ti_cmd(cmdlen);
+s->ti_size = 0;
+if (esp_get_tc(s) == 0) {
+/* Command has been received */
+do_cmd(s);
+}
+break;
+
case STAT_DO:
if (!s->current_req) {
return;
--
2.39.2
[PATCH 45/88] esp.c: remove do_cmd from ESPState
Now that the accumulation of the CDB is handled by SCSI phase, there is no need
for a separate variable to control it.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 13 -
1 file changed, 13 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index e679b1c39b..1f7dff4ca6 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -380,7 +380,6 @@ static void satn_pdma_cb(ESPState *s)
if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
s->cmdfifo_cdb_offset = 1;
-s->do_cmd = 0;
do_cmd(s);
}
}
@@ -400,13 +399,11 @@ static void handle_satn(ESPState *s)
cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
if (cmdlen > 0) {
s->cmdfifo_cdb_offset = 1;
-s->do_cmd = 0;
do_cmd(s);
} else if (cmdlen == 0) {
if (s->dma) {
esp_raise_drq(s);
}
-s->do_cmd = 1;
/* Target present, but no cmd yet - switch to command phase */
s->rregs[ESP_RSEQ] = SEQ_CD;
esp_set_phase(s, STAT_CD);
@@ -428,13 +425,11 @@ static void handle_s_without_atn(ESPState *s)
cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
if (cmdlen > 0) {
s->cmdfifo_cdb_offset = 0;
-s->do_cmd = 0;
do_cmd(s);
} else if (cmdlen == 0) {
if (s->dma) {
esp_raise_drq(s);
}
-s->do_cmd = 1;
/* Target present, but no cmd yet - switch to command phase */
s->rregs[ESP_RSEQ] = SEQ_CD;
esp_set_phase(s, STAT_CD);
@@ -453,7 +448,6 @@ static void satn_stop_pdma_cb(ESPState *s)
if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
-s->do_cmd = 1;
s->cmdfifo_cdb_offset = 1;
esp_set_phase(s, STAT_CD);
s->rregs[ESP_RSTAT] |= STAT_TC;
@@ -478,7 +472,6 @@ static void handle_satn_stop(ESPState *s)
cmdlen = get_cmd(s, 1);
if (cmdlen > 0) {
trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
-s->do_cmd = 1;
s->cmdfifo_cdb_offset = 1;
esp_set_phase(s, STAT_MO);
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
@@ -488,7 +481,6 @@ static void handle_satn_stop(ESPState *s)
if (s->dma) {
esp_raise_drq(s);
}
-s->do_cmd = 1;
/* Target present, switch to message out phase */
s->rregs[ESP_RSEQ] = SEQ_MO;
esp_set_phase(s, STAT_MO);
@@ -567,7 +559,6 @@ static void do_dma_pdma_cb(ESPState *s)
}
/* Command has been received */
-s->do_cmd = 0;
do_cmd(s);
} else {
/*
@@ -669,7 +660,6 @@ static void esp_do_dma(ESPState *s)
}
/* Command has been received */
-s->do_cmd = 0;
do_cmd(s);
} else {
/*
@@ -805,7 +795,6 @@ static void esp_do_nodma(ESPState *s)
}
/* Command has been received */
-s->do_cmd = 0;
do_cmd(s);
} else {
/*
@@ -949,7 +938,6 @@ void esp_transfer_data(SCSIRequest *req, uint32_t len)
int to_device = (esp_get_phase(s) == STAT_DO);
uint32_t dmalen = esp_get_tc(s);
-assert(!s->do_cmd);
trace_esp_transfer_data(dmalen, s->ti_size);
s->async_len = len;
s->async_buf = scsi_req_get_buf(req);
@@ -1012,7 +1000,6 @@ void esp_hard_reset(ESPState *s)
fifo8_reset(&s->fifo);
fifo8_reset(&s->cmdfifo);
s->dma = 0;
-s->do_cmd = 0;
s->dma_cb = NULL;
s->rregs[ESP_CFG1] = 7;
--
2.39.2
[PATCH 33/88] esp.c: copy logic for do_cmd transfers from do_dma_pdma_cb() to esp_do_dma()
This is so that PDMA transfers can be performend by esp_do_dma() as well as
do_dma_pdma_cb().
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 65b4baab83..14284ef54a 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -656,9 +656,18 @@ static void esp_do_dma(ESPState *s)
fifo8_push_all(&s->cmdfifo, buf, len);
esp_set_tc(s, esp_get_tc(s) - len);
} else {
+n = esp_fifo_pop_buf(&s->fifo, buf, fifo8_num_used(&s->fifo));
+n = MIN(fifo8_num_free(&s->cmdfifo), n);
+fifo8_push_all(&s->cmdfifo, buf, n);
+esp_set_tc(s, esp_get_tc(s) - n);
+
esp_set_pdma_cb(s, DO_DMA_PDMA_CB);
esp_raise_drq(s);
-return;
+
+/* Ensure we have received complete command after SATN and stop */
+if (esp_get_tc(s) || fifo8_is_empty(&s->cmdfifo)) {
+return;
+}
}
trace_esp_handle_ti_cmd(cmdlen);
s->ti_size = 0;
--
2.39.2
[PATCH 01/88] esp: don't clear cmdfifo when esp_select() fails in get_cmd()
The FIFO contents should not be affected if the target selection fails.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 3a1c9f7c3b..68d07edc05 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -252,11 +252,9 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
fifo8_push_all(&s->cmdfifo, buf, dmalen);
} else {
if (esp_select(s) < 0) {
-fifo8_reset(&s->cmdfifo);
return -1;
}
esp_raise_drq(s);
-fifo8_reset(&s->cmdfifo);
return 0;
}
} else {
@@ -271,7 +269,6 @@ static uint32_t get_cmd(ESPState *s, uint32_t maxlen)
trace_esp_get_cmd(dmalen, target);
if (esp_select(s) < 0) {
-fifo8_reset(&s->cmdfifo);
return -1;
}
return dmalen;
--
2.39.2
[PATCH 64/88] esp.c: don't raise INTR_BS interrupt in DATA IN phase until TI command issued
In the case where a SCSI command with a DATA IN phase has been issued, the host
may preload the FIFO with unaligned bytes before issuing the main DMA transfer.
When accumulating data in the FIFO don't raise the INTR_BS interrupt until the
TI command is issued, otherwise the unexpected interrupt can confuse the host.
In particular this is needed to prevent the MacOS Disk Utility from failing
when switching non-DMA transfers to use esp_do_nodma().
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index bcebd00831..dd6bf6f033 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -786,6 +786,11 @@ static void esp_do_nodma(ESPState *s)
return;
}
+/* If preloading the FIFO, defer until TI command issued */
+if (s->rregs[ESP_CMD] != CMD_TI) {
+return;
+}
+
s->rregs[ESP_RINTR] |= INTR_BS;
esp_raise_irq(s);
break;
--
2.39.2
