Re: [PATCH] tests/qtest/libqos/e1000e: Refer common PCI ID definitions
On 3/11/22 02:50, Akihiko Odaki wrote:
This is yet another minor cleanup to ease understanding and
future refactoring of the tests.
Signed-off-by: Akihiko Odaki
---
tests/qtest/libqos/e1000e.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c
index 2ea5db65d8..1f2b8f 100644
--- a/tests/qtest/libqos/e1000e.c
+++ b/tests/qtest/libqos/e1000e.c
@@ -18,6 +18,7 @@
#include "qemu/osdep.h"
#include "hw/net/e1000_regs.h"
+#include "hw/pci/pci_ids.h"
#include "../libqtest.h"
#include "pci-pc.h"
#include "qemu/sockets.h"
@@ -217,8 +218,8 @@ static void *e1000e_pci_create(void *pci_bus,
QGuestAllocator *alloc,
static void e1000e_register_nodes(void)
{
QPCIAddress addr = {
-.vendor_id = 0x8086,
-.device_id = 0x10D3,
+.vendor_id = PCI_VENDOR_ID_INTEL,
+.device_id = E1000_DEV_ID_82574L,
};
/* FIXME: every test using this node needs to setup a -netdev socket,id=hs0
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 3/3] vdpa: Expose VIRTIO_NET_F_STATUS unconditionally
On Thu, Nov 3, 2022 at 4:21 AM Jason Wang wrote:
>
> On Wed, Nov 2, 2022 at 7:19 PM Eugenio Perez Martin
> wrote:
> >
> > On Tue, Nov 1, 2022 at 9:10 AM Jason Wang wrote:
> > >
> > > On Fri, Oct 28, 2022 at 5:30 PM Eugenio Perez Martin
> > > wrote:
> > > >
> > > > On Fri, Oct 28, 2022 at 3:59 AM Jason Wang wrote:
> > > > >
> > > > > On Thu, Oct 27, 2022 at 6:18 PM Eugenio Perez Martin
> > > > > wrote:
> > > > > >
> > > > > > On Thu, Oct 27, 2022 at 8:54 AM Jason Wang
> > > > > > wrote:
> > > > > > >
> > > > > > > On Thu, Oct 27, 2022 at 2:47 PM Eugenio Perez Martin
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Thu, Oct 27, 2022 at 6:32 AM Jason Wang
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > 在 2022/10/26 17:53, Eugenio Pérez 写道:
> > > > > > > > > > Now that qemu can handle and emulate it if the vdpa backend
> > > > > > > > > > does not
> > > > > > > > > > support it we can offer it always.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Eugenio Pérez
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > I may miss something but isn't more easier to simply remove
> > > > > > > > > the
> > > > > > > > > _F_STATUS from vdpa_feature_bits[]?
> > > > > > > > >
> > > > > > > >
> > > > > > > > How is that? if we remove it, the guest cannot ack it so it
> > > > > > > > cannot
> > > > > > > > access the net status, isn't it?
> > > > > > >
> > > > > > > My understanding is that the bits stored in the
> > > > > > > vdpa_feature_bits[]
> > > > > > > are the features that must be explicitly supported by the vhost
> > > > > > > device.
> > > > > >
> > > > > > (Non English native here, so maybe I don't get what you mean :) )
> > > > > > The
> > > > > > device may not support them. net simulator lacks some of them
> > > > > > actually, and it works.
> > > > >
> > > > > Speaking too fast, I think I meant that, if the bit doesn't belong to
> > > > > vdpa_feature_bits[], it is assumed to be supported by the Qemu without
> > > > > the support of the vhost. So Qemu won't even try to validate if vhost
> > > > > has this support. E.g for vhost-net, we only have:
> > > > >
> > > > > static const int kernel_feature_bits[] = {
> > > > > VIRTIO_F_NOTIFY_ON_EMPTY,
> > > > > VIRTIO_RING_F_INDIRECT_DESC,
> > > > > VIRTIO_RING_F_EVENT_IDX,
> > > > > VIRTIO_NET_F_MRG_RXBUF,
> > > > > VIRTIO_F_VERSION_1,
> > > > > VIRTIO_NET_F_MTU,
> > > > > VIRTIO_F_IOMMU_PLATFORM,
> > > > > VIRTIO_F_RING_PACKED,
> > > > > VIRTIO_NET_F_HASH_REPORT,
> > > > > VHOST_INVALID_FEATURE_BIT
> > > > > };
> > > > >
> > > > > You can see there's no STATUS bit there since it is emulated by Qemu.
> > > > >
> > > >
> > > > Ok now I get what you mean, and yes we may modify the patches in that
> > > > direction.
> > > >
> > > > But if we go then we need to modify how qemu ack the features, because
> > > > the features that are not in vdpa_feature_bits are not acked to the
> > > > device. More on this later.
> > > >
> > > > > >
> > > > > > From what I see these are the only features that will be forwarded
> > > > > > to
> > > > > > the guest as device_features. If it is not in the list, the feature
> > > > > > will be masked out,
> > > > >
> > > > > Only when there's no support for this feature from the vhost.
> > > > >
> > > > > > as if the device does not support it.
> > > > > >
> > > > > > So now _F_STATUS it was forwarded only if the device supports it. If
> > > > > > we remove it from bit_mask, it will never be offered to the guest.
> > > > > > But
> > > > > > we want to offer it always, since we will need it for
> > > > > > _F_GUEST_ANNOUNCE.
> > > > > >
> > > > > > Things get more complex because we actually need to ack it back if
> > > > > > the
> > > > > > device offers it, so the vdpa device can report link_down. We will
> > > > > > only emulate LINK_UP always in the case the device does not support
> > > > > > _F_STATUS.
> > > > > >
> > > > > > > So if we remove _F_STATUS, Qemu vhost code won't validate if
> > > > > > > vhost-vdpa device has this support:
> > > > > > >
> > > > > > > uint64_t vhost_get_features(struct vhost_dev *hdev, const int
> > > > > > > *feature_bits,
> > > > > > > uint64_t features)
> > > > > > > {
> > > > > > > const int *bit = feature_bits;
> > > > > > > while (*bit != VHOST_INVALID_FEATURE_BIT) {
> > > > > > > uint64_t bit_mask = (1ULL << *bit);
> > > > > > > if (!(hdev->features & bit_mask)) {
> > > > > > > features &= ~bit_mask;
> > > > > > > }
> > > > > > > bit++;
> > > > > > > }
> > > > > > > return features;
> > > > > > > }
> > > > > > >
> > > > > >
> > > > > > Now maybe I'm the one missing something, but why is this not done
> > > > > > as a
> > > > > > masking directly?
> > > > >
> > > > > Not sure, the code has been there since day 0.
> > > > >
> > > > > But you can see from the code:
> > > > >
> > > > > 1) if STATUS is in featu
Re: [PATCH] i386/cpu: Adjust cpuid addresable id count to match the spec
I'm not aware of any bug reports. L1 cache is typically shared between
logical threads, so it seemed reasonable to correct this.
On Fri, Oct 28, 2022 at 9:54 AM Wang, Lei wrote:
>
> On 10/10/2022 10:49 AM, Ilya Oleinik wrote:
> > For EBX bits 23 - 16 in CPUID[01] Intel's manual states:
> > "
> > * The nearest power-of-2 integer that is not smaller than EBX[23:16]
> > is the number of unique initial APICIDs reserved for addressing
> > different logical processors in a physical package. This field is
> > only valid if CPUID.1.EDX.HTT[bit 28]= 1.
> > "
> > Ensure this condition is met.
> >
> > Additionally, apply efb3934adf9ee7794db7e0ade9f576c634592891 to
> > non passthrough cache mode.
> >
> > Signed-off-by: Ilya Oleinik
> > ---
> > target/i386/cpu.c | 10 +-
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index ad623d91e4..e793bcc03f 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -245,8 +245,8 @@ static void encode_cache_cpuid4(CPUCacheInfo *cache,
> > *eax = CACHE_TYPE(cache->type) |
> > CACHE_LEVEL(cache->level) |
> > (cache->self_init ? CACHE_SELF_INIT_LEVEL : 0) |
> > - ((num_cores - 1) << 26) |
> > - ((num_apic_ids - 1) << 14);
> > + ((pow2ceil(num_cores) - 1) << 26) |
> > + ((pow2ceil(num_apic_ids) - 1) << 14);
> >
> > assert(cache->line_size > 0);
> > assert(cache->partitions > 0);
> > @@ -5258,7 +5258,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
> index, uint32_t count,
> > }
> > *edx = env->features[FEAT_1_EDX];
> > if (cs->nr_cores * cs->nr_threads > 1) {
> > -*ebx |= (cs->nr_cores * cs->nr_threads) << 16;
> > +*ebx |= (pow2ceil(cs->nr_cores * cs->nr_threads)) << 16;
> > *edx |= CPUID_HT;
> > }
> > if (!cpu->enable_pmu) {
> > @@ -5313,12 +5313,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
> index, uint32_t count,
> > switch (count) {
> > case 0: /* L1 dcache info */
> > encode_cache_cpuid4(env->cache_info_cpuid4.l1d_cache,
> > -1, cs->nr_cores,
>
> Hi Ilya,
>
> Just curious why the origin implementation is hard-coded to 1 and is there
> any
> bug reported related to this?
>
> > +cs->nr_threads, cs->nr_cores,
> > eax, ebx, ecx, edx);
> > break;
> > case 1: /* L1 icache info */
> > encode_cache_cpuid4(env->cache_info_cpuid4.l1i_cache,
> > -1, cs->nr_cores,
> > +cs->nr_threads, cs->nr_cores,
> > eax, ebx, ecx, edx);
> > break;
> > case 2: /* L2 cache info */
>
[PATCH] tests/qtest/libqos/e1000e: Use E1000_STATUS_ASDV_1000
Nemonics E1000_STATUS_LAN_INIT_DONE and E1000_STATUS_ASDV_1000 have the same value, and E1000_STATUS_ASDV_1000 should be used here because E1000_STATUS_ASDV_1000 represents the auto-detected speed tested here while E1000_STATUS_LAN_INIT_DONE is a value used for a different purpose with a variant of e1000e family different from the one implemented in QEMU. Signed-off-by: Akihiko Odaki --- tests/qtest/libqos/e1000e.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/qtest/libqos/e1000e.c b/tests/qtest/libqos/e1000e.c index 11d9f55c66..178d61d04f 100644 --- a/tests/qtest/libqos/e1000e.c +++ b/tests/qtest/libqos/e1000e.c @@ -130,8 +130,8 @@ static void e1000e_pci_start_hw(QOSGraphObject *obj) /* Check the device status - link and speed */ val = e1000e_macreg_read(&d->e1000e, E1000_STATUS); -g_assert_cmphex(val & (E1000_STATUS_LU | E1000_STATUS_LAN_INIT_DONE), -==, E1000_STATUS_LU | E1000_STATUS_LAN_INIT_DONE); +g_assert_cmphex(val & (E1000_STATUS_LU | E1000_STATUS_ASDV_1000), +==, E1000_STATUS_LU | E1000_STATUS_ASDV_1000); /* Initialize TX/RX logic */ e1000e_macreg_write(&d->e1000e, E1000_RCTL, 0); -- 2.38.1
[PATCH] Add missing include statement for global xml_builtin
This fixes some compiler warnings with compiler flag
-Wmissing-variable-declarations (tested with clang):
aarch64_be-linux-user-gdbstub-xml.c:564:19: warning: no previous extern
declaration for non-static variable 'xml_builtin'
[-Wmissing-variable-declarations]
aarch64-linux-user-gdbstub-xml.c:564:19: warning: no previous extern
declaration for non-static variable 'xml_builtin'
[-Wmissing-variable-declarations]
aarch64-softmmu-gdbstub-xml.c:1763:19: warning: no previous extern
declaration for non-static variable 'xml_builtin'
[-Wmissing-variable-declarations]
Signed-off-by: Stefan Weil
---
scripts/feature_to_c.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/scripts/feature_to_c.sh b/scripts/feature_to_c.sh
index b1169899c1..c1f67c8f6a 100644
--- a/scripts/feature_to_c.sh
+++ b/scripts/feature_to_c.sh
@@ -56,6 +56,7 @@ for input; do
done
echo
+echo '#include "exec/gdbstub.h"'
echo "const char *const xml_builtin[][2] = {"
for input; do
--
2.30.2
[PULL 2/4] linux-user: Add close_range() syscall
From: Helge Deller
Signed-off-by: Helge Deller
Reviewed-by: Richard Henderson
Message-Id:
Signed-off-by: Laurent Vivier
---
linux-user/strace.list | 3 +++
linux-user/syscall.c | 19 +++
2 files changed, 22 insertions(+)
diff --git a/linux-user/strace.list b/linux-user/strace.list
index 3df2184580aa..cd995e5d56db 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -103,6 +103,9 @@
#ifdef TARGET_NR_close
{ TARGET_NR_close, "close" , "%s(%d)", NULL, NULL },
#endif
+#ifdef TARGET_NR_close_range
+{ TARGET_NR_close_range, "close_range" , "%s(%u,%u,%u)", NULL, NULL },
+#endif
#ifdef TARGET_NR_connect
{ TARGET_NR_connect, "connect" , "%s(%d,%#x,%d)", NULL, NULL },
#endif
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8402c1399d3c..8b18adfba894 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -364,6 +364,13 @@ _syscall3(int,sys_syslog,int,type,char*,bufp,int,len)
#ifdef __NR_exit_group
_syscall1(int,exit_group,int,error_code)
#endif
+#if defined(__NR_close_range) && defined(TARGET_NR_close_range)
+#define __NR_sys_close_range __NR_close_range
+_syscall3(int,sys_close_range,int,first,int,last,int,flags)
+#ifndef CLOSE_RANGE_CLOEXEC
+#define CLOSE_RANGE_CLOEXEC (1U << 2)
+#endif
+#endif
#if defined(__NR_futex)
_syscall6(int,sys_futex,int *,uaddr,int,op,int,val,
const struct timespec *,timeout,int *,uaddr2,int,val3)
@@ -8756,6 +8763,18 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int
num, abi_long arg1,
case TARGET_NR_close:
fd_trans_unregister(arg1);
return get_errno(close(arg1));
+#if defined(__NR_close_range) && defined(TARGET_NR_close_range)
+case TARGET_NR_close_range:
+ret = get_errno(sys_close_range(arg1, arg2, arg3));
+if (ret == 0 && !(arg3 & CLOSE_RANGE_CLOEXEC)) {
+abi_long fd, maxfd;
+maxfd = MIN(arg2, target_fd_max);
+for (fd = arg1; fd < maxfd; fd++) {
+fd_trans_unregister(fd);
+}
+}
+return ret;
+#endif
case TARGET_NR_brk:
return do_brk(arg1);
--
2.37.3
[PULL 0/4] Linux user for 7.2 patches
The following changes since commit a11f65ec1b8adcb012b89c92819cbda4dc25aaf1: Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2022-11-01 13:49:33 -0400) are available in the Git repository at: https://gitlab.com/laurent_vivier/qemu.git tags/linux-user-for-7.2-pull-request for you to fetch changes up to 16c81dd563b94e9392a578ccf5aa762d01e8f165: linux-user: always translate cmsg when recvmsg (2022-11-02 17:29:17 +0100) linux-user pull request 20221103 Fix recvmsg Fix hppa exception handler Add close_range Add strace for timer_settime64 Helge Deller (3): linux-user/hppa: Detect glibc ABORT_INSTRUCTION and EXCP_BREAK handler linux-user: Add close_range() syscall linux-user: Add strace output for timer_settime64() syscall Icenowy Zheng (1): linux-user: always translate cmsg when recvmsg linux-user/hppa/cpu_loop.c | 19 ++- linux-user/strace.list | 8 +++- linux-user/syscall.c | 22 +- 3 files changed, 42 insertions(+), 7 deletions(-) -- 2.37.3
[PULL 4/4] linux-user: always translate cmsg when recvmsg
From: Icenowy Zheng
It's possible that a message contains both normal payload and ancillary
data in the same message, and even if no ancillary data is available
this information should be passed to the target, otherwise the target
cmsghdr will be left uninitialized and the target is going to access
uninitialized memory if it expects cmsg.
Always call the function that translate cmsg when recvmsg, because that
function should be empty-cmsg-safe (it creates an empty cmsg in the
target).
Signed-off-by: Icenowy Zheng
Reviewed-by: Laurent Vivier
Message-Id: <20221028081220.1604244-1-...@icenowy.me>
Signed-off-by: Laurent Vivier
---
linux-user/syscall.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 8b18adfba894..24b25759beab 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -3353,7 +3353,8 @@ static abi_long do_sendrecvmsg_locked(int fd, struct
target_msghdr *msgp,
if (fd_trans_host_to_target_data(fd)) {
ret = fd_trans_host_to_target_data(fd)(msg.msg_iov->iov_base,
MIN(msg.msg_iov->iov_len, len));
-} else {
+}
+if (!is_error(ret)) {
ret = host_to_target_cmsg(msgp, &msg);
}
if (!is_error(ret)) {
--
2.37.3
[PULL 1/4] linux-user/hppa: Detect glibc ABORT_INSTRUCTION and EXCP_BREAK handler
From: Helge Deller
The glibc on the hppa platform uses the "iitlbp %r0,(%sr0, %r0)"
assembler instruction as ABORT_INSTRUCTION.
If this (in userspace context) illegal assembler statement is found,
dump the registers and report the failure to userspace the same way as
the Linux kernel on physical hardware.
For other illegal instructions report TARGET_ILL_ILLOPC instead of
TARGET_ILL_ILLOPN as si_code.
Additionally add the missing EXCP_BREAK exception handler which occurs
when the "break x,y" assembler instruction is executed and report
EXCP_ASSIST traps.
Signed-off-by: Helge Deller
Message-Id:
Signed-off-by: Laurent Vivier
---
linux-user/hppa/cpu_loop.c | 19 ++-
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
index 1ef3b461911c..8ab133510602 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -147,15 +147,20 @@ void cpu_loop(CPUHPPAState *env)
force_sig_fault(TARGET_SIGSEGV, TARGET_SEGV_MAPERR, env->iaoq_f);
break;
case EXCP_ILL:
-EXCP_DUMP(env, "qemu: got CPU exception 0x%x - aborting\n",
trapnr);
-force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPN, env->iaoq_f);
+EXCP_DUMP(env, "qemu: EXCP_ILL exception %#x\n", trapnr);
+force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPC, env->iaoq_f);
break;
case EXCP_PRIV_OPR:
-EXCP_DUMP(env, "qemu: got CPU exception 0x%x - aborting\n",
trapnr);
-force_sig_fault(TARGET_SIGILL, TARGET_ILL_PRVOPC, env->iaoq_f);
+/* check for glibc ABORT_INSTRUCTION "iitlbp %r0,(%sr0, %r0)" */
+EXCP_DUMP(env, "qemu: EXCP_PRIV_OPR exception %#x\n", trapnr);
+if (env->cr[CR_IIR] == 0x0400) {
+ force_sig_fault(TARGET_SIGILL, TARGET_ILL_ILLOPC,
env->iaoq_f);
+} else {
+ force_sig_fault(TARGET_SIGILL, TARGET_ILL_PRVOPC,
env->iaoq_f);
+}
break;
case EXCP_PRIV_REG:
-EXCP_DUMP(env, "qemu: got CPU exception 0x%x - aborting\n",
trapnr);
+EXCP_DUMP(env, "qemu: EXCP_PRIV_REG exception %#x\n", trapnr);
force_sig_fault(TARGET_SIGILL, TARGET_ILL_PRVREG, env->iaoq_f);
break;
case EXCP_OVERFLOW:
@@ -167,6 +172,10 @@ void cpu_loop(CPUHPPAState *env)
case EXCP_ASSIST:
force_sig_fault(TARGET_SIGFPE, 0, env->iaoq_f);
break;
+case EXCP_BREAK:
+EXCP_DUMP(env, "qemu: EXCP_BREAK exception %#x\n", trapnr);
+force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->iaoq_f &
~3);
+break;
case EXCP_DEBUG:
force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->iaoq_f);
break;
--
2.37.3
[PULL 3/4] linux-user: Add strace output for timer_settime64() syscall
From: Helge Deller
Add missing timer_settime64() strace output and specify format for
timer_settime().
Signed-off-by: Helge Deller
Message-Id:
Signed-off-by: Laurent Vivier
---
linux-user/strace.list | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/linux-user/strace.list b/linux-user/strace.list
index cd995e5d56db..3a898e2532d3 100644
--- a/linux-user/strace.list
+++ b/linux-user/strace.list
@@ -1534,7 +1534,10 @@
{ TARGET_NR_timer_gettime, "timer_gettime" , NULL, NULL, NULL },
#endif
#ifdef TARGET_NR_timer_settime
-{ TARGET_NR_timer_settime, "timer_settime" , NULL, NULL, NULL },
+{ TARGET_NR_timer_settime, "timer_settime" , "%s(%d,%d,%p,%p)", NULL, NULL },
+#endif
+#ifdef TARGET_NR_timer_settime64
+{ TARGET_NR_timer_settime64, "timer_settime64" , "%s(%d,%d,%p,%p)", NULL, NULL
},
#endif
#ifdef TARGET_NR_timerfd
{ TARGET_NR_timerfd, "timerfd" , NULL, NULL, NULL },
--
2.37.3
Re: [PATCH for 7.2] Fix broken configure with -Wunused-parameter
On Wed, Nov 02, 2022 at 09:22:58PM +0100, Stefan Weil via wrote:
> The configure script fails because it tries to compile small C programs
> with a main function which is declared with arguments argc and argv
> although those arguments are unused.
>
> Running `configure -extra-cflags=-Wunused-parameter` triggers the problem.
> configure for a native build does abort but shows the error in config.log.
> A cross build configure for Windows with Debian stable aborts with an
> error.
>
> Avoiding unused arguments fixes this.
I'm not convinced that we should allow -extra-cflags to influence
the configure compile checks at all, as there are likely more cases
where arbitrary -W$warn flag will impact the checks, potentially
causing configure to silently take the wrong action.
>
> Signed-off-by: Stefan Weil
> ---
>
> See https://gitlab.com/qemu-project/qemu/-/issues/1295.
>
> I noticed the problem because I often compile with -Wextra.
>
> Stefan
>
> configure | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 4275f5419f..1106c04fea 100755
> --- a/configure
> +++ b/configure
> @@ -1258,6 +1258,7 @@ if test "$stack_protector" != "no"; then
>cat > $TMPC << EOF
> int main(int argc, char *argv[])
> {
> +(void)argc;
> char arr[64], *p = arr, *c = argv[0];
> while (*c) {
> *p++ = *c++;
> @@ -1607,7 +1608,7 @@ fi
>
> if test "$safe_stack" = "yes"; then
> cat > $TMPC << EOF
> -int main(int argc, char *argv[])
> +int main(void)
> {
> #if ! __has_feature(safe_stack)
> #error SafeStack Disabled
> @@ -1629,7 +1630,7 @@ EOF
>fi
> else
> cat > $TMPC << EOF
> -int main(int argc, char *argv[])
> +int main(void)
> {
> #if defined(__has_feature)
> #if __has_feature(safe_stack)
> @@ -1675,7 +1676,7 @@ static const int Z = 1;
> #define TAUT(X) ((X) == Z)
> #define PAREN(X, Y) (X == Y)
> #define ID(X) (X)
> -int main(int argc, char *argv[])
> +int main(void)
> {
> int x = 0, y = 0;
> x = ID(x);
> --
> 2.30.2
>
>
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 for 7.2] Fix broken configure with -Wunused-parameter
Am 03.11.22 um 09:58 schrieb Daniel P. Berrangé: On Wed, Nov 02, 2022 at 09:22:58PM +0100, Stefan Weil via wrote: The configure script fails because it tries to compile small C programs with a main function which is declared with arguments argc and argv although those arguments are unused. Running `configure -extra-cflags=-Wunused-parameter` triggers the problem. configure for a native build does abort but shows the error in config.log. A cross build configure for Windows with Debian stable aborts with an error. Avoiding unused arguments fixes this. I'm not convinced that we should allow -extra-cflags to influence the configure compile checks at all, as there are likely more cases where arbitrary -W$warn flag will impact the checks, potentially causing configure to silently take the wrong action. I partially agree, but configure should fail if invalid -extra-cflags are specified, and the checks must also respect additional include paths given by -extra-cflags of course. And I think that the changes in my patch are an improvement in any case. Stefan OpenPGP_0xE08C21D5677450AD.asc Description: OpenPGP public key OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] migration: check magic value for deciding the mapping of channels
On 01/11/22 9:15 pm, Daniel P. Berrangé wrote:
On Tue, Nov 01, 2022 at 09:10:14PM +0530, manish.mishra wrote:
On 01/11/22 8:21 pm, Daniel P. Berrangé wrote:
On Tue, Nov 01, 2022 at 02:30:29PM +, manish.mishra wrote:
Current logic assumes that channel connections on the destination side are
always established in the same order as the source and the first one will
always be the default channel followed by the multifid or post-copy
preemption channel. This may not be always true, as even if a channel has a
connection established on the source side it can be in the pending state on
the destination side and a newer connection can be established first.
Basically causing out of order mapping of channels on the destination side.
Currently, all channels except post-copy preempt send a magic number, this
patch uses that magic number to decide the type of channel. This logic is
applicable only for precopy(multifd) live migration, as mentioned, the
post-copy preempt channel does not send any magic number. Also, this patch
uses MSG_PEEK to check the magic number of channels so that current
data/control stream management remains un-effected.
Signed-off-by: manish.mishra
---
include/io/channel.h | 25 +
io/channel-socket.c | 27 +++
io/channel.c | 39 +++
migration/migration.c| 33 +
migration/multifd.c | 12
migration/multifd.h | 2 +-
migration/postcopy-ram.c | 5 +
migration/postcopy-ram.h | 2 +-
8 files changed, 119 insertions(+), 26 deletions(-)
diff --git a/include/io/channel.h b/include/io/channel.h
index c680ee7480..74177aeeea 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -115,6 +115,10 @@ struct QIOChannelClass {
int **fds,
size_t *nfds,
Error **errp);
+ssize_t (*io_read_peek)(QIOChannel *ioc,
+void *buf,
+size_t nbytes,
+Error **errp);
int (*io_close)(QIOChannel *ioc,
Error **errp);
GSource * (*io_create_watch)(QIOChannel *ioc,
@@ -475,6 +479,27 @@ int qio_channel_write_all(QIOChannel *ioc,
size_t buflen,
Error **errp);
+/**
+ * qio_channel_read_peek_all:
+ * @ioc: the channel object
+ * @buf: the memory region to read in data
+ * @nbytes: the number of bytes to read
+ * @errp: pointer to a NULL-initialized error object
+ *
+ * Read given @nbytes data from peek of channel into
+ * memory region @buf.
+ *
+ * The function will be blocked until read size is
+ * equal to requested size.
+ *
+ * Returns: 1 if all bytes were read, 0 if end-of-file
+ * occurs without data, or -1 on error
+ */
+int qio_channel_read_peek_all(QIOChannel *ioc,
+ void* buf,
+ size_t nbytes,
+ Error **errp);
+
/**
* qio_channel_set_blocking:
* @ioc: the channel object
diff --git a/io/channel-socket.c b/io/channel-socket.c
index b76dca9cc1..b99f5dfda6 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -705,6 +705,32 @@ static ssize_t qio_channel_socket_writev(QIOChannel *ioc,
}
#endif /* WIN32 */
+static ssize_t qio_channel_socket_read_peek(QIOChannel *ioc,
+void *buf,
+size_t nbytes,
+Error **errp)
+{
+QIOChannelSocket *sioc = QIO_CHANNEL_SOCKET(ioc);
+ssize_t bytes = 0;
+
+retry:
+bytes = recv(sioc->fd, buf, nbytes, MSG_PEEK);
+
+if (bytes < 0) {
+if (errno == EINTR) {
+goto retry;
+}
+if (errno == EAGAIN) {
+return QIO_CHANNEL_ERR_BLOCK;
+}
+
+error_setg_errno(errp, errno,
+ "Unable to read from peek of socket");
+return -1;
+}
+
+return bytes;
+}
#ifdef QEMU_MSG_ZEROCOPY
static int qio_channel_socket_flush(QIOChannel *ioc,
@@ -902,6 +928,7 @@ static void qio_channel_socket_class_init(ObjectClass
*klass,
ioc_klass->io_writev = qio_channel_socket_writev;
ioc_klass->io_readv = qio_channel_socket_readv;
+ioc_klass->io_read_peek = qio_channel_socket_read_peek;
ioc_klass->io_set_blocking = qio_channel_socket_set_blocking;
ioc_klass->io_close = qio_channel_socket_close;
ioc_klass->io_shutdown = qio_channel_socket_shutdown;
diff --git a/io/channel.c b/io/channel.c
index 0640941ac5..a2d9b96f3f 100644
--- a/io/channel.c
+++ b/io/channel.c
@@ -346,6 +346,45 @@ int qio_channel_write_all(QIOChannel *ioc,
return qio_channel_writev_all(ioc, &iov, 1, errp);
}
+int qio_channel_read_peek_all(QIOChannel *ioc,
+ void* buf,
+
Re: [PATCH] migration: check magic value for deciding the mapping of channels
On Thu, Nov 03, 2022 at 02:50:25PM +0530, manish.mishra wrote:
>
> On 01/11/22 9:15 pm, Daniel P. Berrangé wrote:
> > On Tue, Nov 01, 2022 at 09:10:14PM +0530, manish.mishra wrote:
> > > On 01/11/22 8:21 pm, Daniel P. Berrangé wrote:
> > > > On Tue, Nov 01, 2022 at 02:30:29PM +, manish.mishra wrote:
> > > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > > index 739bb683f3..f4b6f278a9 100644
> > > > > --- a/migration/migration.c
> > > > > +++ b/migration/migration.c
> > > > > @@ -733,31 +733,40 @@ void migration_ioc_process_incoming(QIOChannel
> > > > > *ioc, Error **errp)
> > > > >{
> > > > >MigrationIncomingState *mis = migration_incoming_get_current();
> > > > >Error *local_err = NULL;
> > > > > -bool start_migration;
> > > > >QEMUFile *f;
> > > > > +bool default_channel = true;
> > > > > +uint32_t channel_magic = 0;
> > > > > +int ret = 0;
> > > > > -if (!mis->from_src_file) {
> > > > > -/* The first connection (multifd may have multiple) */
> > > > > +if (migrate_use_multifd() && !migration_in_postcopy()) {
> > > > > +ret = qio_channel_read_peek_all(ioc, (void *)&channel_magic,
> > > > > +sizeof(channel_magic),
> > > > > &local_err);
> > > > > +
> > > > > +if (ret != 1) {
> > > > > +error_propagate(errp, local_err);
> > > > > +return;
> > > > > +}
> > > > and thus this will fail for TLS channels AFAICT.
> > > Yes, thanks for quick review Daniel. You pointed this earlier too, sorry
> > > missed it, will put another check !migrate_use_tls() in V2.
> > But we need this problem fixed with TLS too, so just excluding it
> > isn't right. IMHO we need to modify the migration code so we can
> > read the magic earlier, instead of peeking.
> >
> >
> > With regards,
> > Daniel
>
> Hi Daniel, I was trying tls migrations. What i see is that tls session
> creation does handshake. So if we read ahead in ioc_process_incoming
> for default channel. Because client sends magic only after multiFD
> channels are setup, which too requires tls handshake.
By the time we get to migrate_ioc_process_incoming, the TLS handshake
has already been performed.
migration_channel_process_incoming
-> migration_ioc_process_incoming
vs
migration_channel_process_incoming
-> migration_tls_channel_process_incoming
-> migration_tls_incoming_handshake
-> migration_channel_process_incoming
-> migration_ioc_process_incoming
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 v14 16/17] tests/qtest: netdev: test stream and dgram backends
On 10/28/22 07:04, Jason Wang wrote:
在 2022/10/21 17:09, Laurent Vivier 写道:
Signed-off-by: Laurent Vivier
Acked-by: Michael S. Tsirkin
---
I got this:
63/63 ERROR:../tests/qtest/netdev-socket.c:139:test_stream_inet_ipv6: assertion failed
(resp == expect): ("st0: index=0,type=stream,connection error\r\n" == "st0:
index=0,type=stream,tcp:::1:40389\r\n") ERROR
63/63 qemu:qtest+qtest-x86_64 / qtest-x86_64/netdev-socket ERROR 5.29s
killed by signal 6 SIGABRT
>>> QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-x86_64 MALLOC_PERTURB_=96
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
G_TEST_DBUS_DAEMON=/home/devel/git/qemu/tests/dbus-vmstate-daemon.sh
/home/devel/git/qemu/build/tests/qtest/netdev-socket --tap -k
――― ✀
―――
stderr:
**
ERROR:../tests/qtest/netdev-socket.c:139:test_stream_inet_ipv6: assertion failed (resp ==
expect): ("st0: index=0,type=stream,connection error\r\n" == "st0:
index=0,type=stream,tcp:::1:40389\r\n")
(test program exited with status code -6)
I'm not able to reproduce the problem.
Is this 100% reproducible?
Is IPv6 enabled on your test machine?
Thanks,
Laurent
Re: [PATCH] tests/unit/test-io-channel-command: Silence GCC error "maybe-uninitialized"
Le 02/11/2022 à 21:24, Alex Bennée a écrit :
Bernhard Beschow writes:
GCC issues a false positive warning, resulting in build failure with -Werror:
In file included from /usr/lib/glib-2.0/include/glibconfig.h:9,
from /usr/include/glib-2.0/glib/gtypes.h:34,
from /usr/include/glib-2.0/glib/galloca.h:34,
from /usr/include/glib-2.0/glib.h:32,
from ../src/include/glib-compat.h:32,
from ../src/include/qemu/osdep.h:144,
from ../src/tests/unit/test-io-channel-command.c:21:
/usr/include/glib-2.0/glib/gmacros.h: In function
‘test_io_channel_command_fifo’:
/usr/include/glib-2.0/glib/gmacros.h:1333:105: error: ‘dstargv’ may be used
uninitialized [-Werror=maybe-uninitialized]
1333 | static G_GNUC_UNUSED inline void _GLIB_AUTO_FUNC_NAME(TypeName)
(TypeName *_ptr) { if (*_ptr != none) (func) (*_ptr); } \
|
^
../src/tests/unit/test-io-channel-command.c:39:19: note: ‘dstargv’ was
declared here
39 | g_auto(GStrv) dstargv;
| ^~~
/usr/include/glib-2.0/glib/gmacros.h:1333:105: error: ‘srcargv’ may
be used uninitialized [-Werror=maybe-uninitialized]
1333 | static G_GNUC_UNUSED inline void
_GLIB_AUTO_FUNC_NAME(TypeName) (TypeName *_ptr) { if (*_ptr != none)
(func) (*_ptr); } \
|
^
../src/tests/unit/test-io-channel-command.c:38:19: note: ‘srcargv’ was
declared here
38 | g_auto(GStrv) srcargv;
| ^~~
cc1: all warnings being treated as errors
GCC version:
$ gcc --version
gcc (GCC) 12.2.0
Fixes: 68406d10859385c88da73d0106254a7f47e6652e ('tests/unit: cleanups for
test-io-channel-command')
Signed-off-by: Bernhard Beschow
---
tests/unit/test-io-channel-command.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/unit/test-io-channel-command.c
b/tests/unit/test-io-channel-command.c
index 43e29c8cfb..ba0717d3c3 100644
--- a/tests/unit/test-io-channel-command.c
+++ b/tests/unit/test-io-channel-command.c
@@ -35,8 +35,8 @@ static void test_io_channel_command_fifo(bool async)
g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
g_autoptr(GString) srcargs = g_string_new(socat);
g_autoptr(GString) dstargs = g_string_new(socat);
-g_auto(GStrv) srcargv;
-g_auto(GStrv) dstargv;
+g_auto(GStrv) srcargv = NULL;
+g_auto(GStrv) dstargv = NULL;
QIOChannel *src, *dst;
QIOChannelTest *test;
Another approach would be to drop the GString usage which is premature
and then we can allocate everything in order:
Yes, it looks like a better approach. I'm going to drop this patch from the
trivial branch.
Could you send a patch?
Thanks,
Laurent
--8<---cut here---start->8---
modified tests/unit/test-io-channel-command.c
@@ -33,19 +33,13 @@ static void test_io_channel_command_fifo(bool async)
{
g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XX",
NULL);
g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
-g_autoptr(GString) srcargs = g_string_new(socat);
-g_autoptr(GString) dstargs = g_string_new(socat);
-g_auto(GStrv) srcargv;
-g_auto(GStrv) dstargv;
+g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat,
fifo);
+g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat,
fifo);
+g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
+g_auto(GStrv) dstargv = g_strsplit(dstargs, " ", -1);
QIOChannel *src, *dst;
QIOChannelTest *test;
-g_string_append_printf(srcargs, " - PIPE:%s,wronly", fifo);
-g_string_append_printf(dstargs, " PIPE:%s,rdonly -", fifo);
-
-srcargv = g_strsplit(srcargs->str, " ", -1);
-dstargv = g_strsplit(dstargs->str, " ", -1);
-
src = QIO_CHANNEL(qio_channel_command_new_spawn((const char **) srcargv,
O_WRONLY,
--8<---cut here---end--->8---
Re: [PATCH 1/2] file-posix: fix Linux alignment probing when EIO is returned
Am 02.11.2022 um 03:49 hat Eric Biggers geschrieben: > On Tue, Nov 01, 2022 at 07:27:16PM -0700, Eric Biggers wrote: > > On Tue, Nov 01, 2022 at 03:00:30PM -0400, Stefan Hajnoczi wrote: > > > Linux dm-crypt returns errno EIO from unaligned O_DIRECT pread(2) calls. > > > > Citation needed. For direct I/O to block devices, the kernel's block layer > > checks the alignment before the I/O is actually submitted to the underlying > > block device. See > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/fops.c?h=v6.1-rc3#n306 > > > > > Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1290 > > > > That "bug" seems to be based on a misunderstanding of the kernel source > > code, > > and not any actual testing. > > > > I just tested it, and the error code is EINVAL. > > > > I think I see what's happening. The kernel code was broken just a few months > ago, in v6.0 by the commit "block: relax direct io memory alignment" > (https://git.kernel.org/linus/b1a000d3b8ec582d). Now the block layer lets DIO > through when the user buffer is only aligned to the device's dma_alignment. > But > a dm-crypt device has a dma_alignment of 512 even when the crypto sector size > (and thus also the logical block size) is 4096. So there is now a case where > misaligned DIO can reach dm-crypt, when that shouldn't be possible. > > It also means that STATX_DIOALIGN will give the wrong value for > stx_dio_mem_align in the above case, 512 instead of 4096. This is because > STATX_DIOALIGN for block devices relies on the dma_alignment. In other words, STATX_DIOALIGN is unusable from the start because we don't know whether the information it returns is actually correct? :-/ I guess we could still use the value returned by STATX_DIOALIGN as a preferred value that we'll use if it survives probing, and otherwise fall back to the same probing we've always been doing because there was no (or no sane) way to get the information from the kernel. > I'll raise this on the linux-block and dm-devel mailing lists. It > would be nice if people reported kernel bugs instead of silently > working around them... I wasn't involved in this specific one, but in case you're wondering why I wouldn't have reported it either... On one hand, I agree with you because I want bugs in my code reported, too, but on the other hand, it has also happened to me before that you're treated as the stupid userland developer who doesn't know how the kernel works and who should better have kept his ideas of how it should work to himself - which is not exactly encouraging to report things when you can just deal with the way they are. I wouldn't have been able to tell whether in the mind of the respective maintainers, -EINVAL is required behaviour or whether that was just a totally unreasonable assumption on our side. Erring on the safe side, I'll give up an assumption that obviously doesn't match reality. And even a kernel fix now doesn't change that there are broken kernels out there and we need to work with them. So reporting it doesn't even solve our problem, it's just additional effort with limited expectations of success. Kevin
[PATCH] tests/qtest/e1000e-test: Use e1000_regs.h
The register definitions in tests/qtest/e1000e-test.c had names
different from hw/net/e1000_regs.h, which made it hard to understand
what test codes corresponds to the implementation. Use
hw/net/e1000_regs.h from tests/qtest/libqos/e1000e.c to remove
these duplications.
Signed-off-by: Akihiko Odaki
---
tests/qtest/e1000e-test.c | 66 ++-
1 file changed, 10 insertions(+), 56 deletions(-)
diff --git a/tests/qtest/e1000e-test.c b/tests/qtest/e1000e-test.c
index c98779c7c0..9e7cb1eb2d 100644
--- a/tests/qtest/e1000e-test.c
+++ b/tests/qtest/e1000e-test.c
@@ -33,34 +33,11 @@
#include "qemu/bitops.h"
#include "libqos/malloc.h"
#include "libqos/e1000e.h"
+#include "hw/net/e1000_regs.h"
static void e1000e_send_verify(QE1000E *d, int *test_sockets, QGuestAllocator
*alloc)
{
-struct {
-uint64_t buffer_addr;
-union {
-uint32_t data;
-struct {
-uint16_t length;
-uint8_t cso;
-uint8_t cmd;
-} flags;
-} lower;
-union {
-uint32_t data;
-struct {
-uint8_t status;
-uint8_t css;
-uint16_t special;
-} fields;
-} upper;
-} descr;
-
-static const uint32_t dtyp_data = BIT(20);
-static const uint32_t dtyp_ext = BIT(29);
-static const uint32_t dcmd_rs = BIT(27);
-static const uint32_t dcmd_eop = BIT(24);
-static const uint32_t dsta_dd = BIT(0);
+struct e1000_tx_desc descr;
static const int data_len = 64;
char buffer[64];
int ret;
@@ -73,10 +50,10 @@ static void e1000e_send_verify(QE1000E *d, int
*test_sockets, QGuestAllocator *a
/* Prepare TX descriptor */
memset(&descr, 0, sizeof(descr));
descr.buffer_addr = cpu_to_le64(data);
-descr.lower.data = cpu_to_le32(dcmd_rs |
- dcmd_eop |
- dtyp_ext |
- dtyp_data |
+descr.lower.data = cpu_to_le32(E1000_TXD_CMD_RS |
+ E1000_TXD_CMD_EOP |
+ E1000_TXD_CMD_DEXT |
+ E1000_TXD_DTYP_D |
data_len);
/* Put descriptor to the ring */
@@ -86,7 +63,8 @@ static void e1000e_send_verify(QE1000E *d, int *test_sockets,
QGuestAllocator *a
e1000e_wait_isr(d, E1000E_TX0_MSG_ID);
/* Check DD bit */
-g_assert_cmphex(le32_to_cpu(descr.upper.data) & dsta_dd, ==, dsta_dd);
+g_assert_cmphex(le32_to_cpu(descr.upper.data) & E1000_TXD_STAT_DD, ==,
+E1000_TXD_STAT_DD);
/* Check data sent to the backend */
ret = recv(test_sockets[0], &recv_len, sizeof(recv_len), 0);
@@ -101,31 +79,7 @@ static void e1000e_send_verify(QE1000E *d, int
*test_sockets, QGuestAllocator *a
static void e1000e_receive_verify(QE1000E *d, int *test_sockets,
QGuestAllocator *alloc)
{
-union {
-struct {
-uint64_t buffer_addr;
-uint64_t reserved;
-} read;
-struct {
-struct {
-uint32_t mrq;
-union {
-uint32_t rss;
-struct {
-uint16_t ip_id;
-uint16_t csum;
-} csum_ip;
-} hi_dword;
-} lower;
-struct {
-uint32_t status_error;
-uint16_t length;
-uint16_t vlan;
-} upper;
-} wb;
-} descr;
-
-static const uint32_t esta_dd = BIT(0);
+union e1000_rx_desc_extended descr;
char test[] = "TEST";
int len = htonl(sizeof(test));
@@ -162,7 +116,7 @@ static void e1000e_receive_verify(QE1000E *d, int
*test_sockets, QGuestAllocator
/* Check DD bit */
g_assert_cmphex(le32_to_cpu(descr.wb.upper.status_error) &
-esta_dd, ==, esta_dd);
+E1000_RXD_STAT_DD, ==, E1000_RXD_STAT_DD);
/* Check data sent to the backend */
memread(data, buffer, sizeof(buffer));
--
2.38.1
Re: [PATCH v7 7/7] hw/arm/virt: Add properties to disable high memory regions
Hi Gavin,
On 10/30/22 00:43, Gavin Shan wrote:
> The 3 high memory regions are usually enabled by default, but they may
> be not used. For example, VIRT_HIGH_GIC_REDIST2 isn't needed by GICv2.
> This leads to waste in the PA space.
>
> Add properties ("highmem-redists", "highmem-ecam", "highmem-mmio") to
> allow users selectively disable them if needed. After that, the high
> memory region for GICv3 or GICv4 redistributor can be disabled by user,
> the number of maximal supported CPUs needs to be calculated based on
> 'vms->highmem_redists'. The follow-up error message is also improved
> to indicate if the high memory region for GICv3 and GICv4 has been
> enabled or not.
>
> Suggested-by: Marc Zyngier
> Signed-off-by: Gavin Shan
> Reviewed-by: Marc Zyngier
Reviewed-by: Eric Auger
Thanks
Eric
> ---
> docs/system/arm/virt.rst | 13 +++
> hw/arm/virt.c| 75 ++--
> 2 files changed, 86 insertions(+), 2 deletions(-)
>
> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> index 4454706392..188a4f211f 100644
> --- a/docs/system/arm/virt.rst
> +++ b/docs/system/arm/virt.rst
> @@ -98,6 +98,19 @@ compact-highmem
>Set ``on``/``off`` to enable/disable the compact layout for high memory
> regions.
>The default is ``on`` for machine types later than ``virt-7.2``.
>
> +highmem-redists
> + Set ``on``/``off`` to enable/disable the high memory region for GICv3 or
> + GICv4 redistributor. The default is ``on``. Setting this to ``off`` will
> + limit the maximum number of CPUs when GICv3 or GICv4 is used.
> +
> +highmem-ecam
> + Set ``on``/``off`` to enable/disable the high memory region for PCI ECAM.
> + The default is ``on`` for machine types later than ``virt-3.0``.
> +
> +highmem-mmio
> + Set ``on``/``off`` to enable/disable the high memory region for PCI MMIO.
> + The default is ``on``.
> +
> gic-version
>Specify the version of the Generic Interrupt Controller (GIC) to provide.
>Valid values are:
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 020a95cfa2..65cff7add1 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2095,14 +2095,20 @@ static void machvirt_init(MachineState *machine)
> if (vms->gic_version == VIRT_GIC_VERSION_2) {
> virt_max_cpus = GIC_NCPU;
> } else {
> -virt_max_cpus = virt_redist_capacity(vms, VIRT_GIC_REDIST) +
> -virt_redist_capacity(vms, VIRT_HIGH_GIC_REDIST2);
> +virt_max_cpus = virt_redist_capacity(vms, VIRT_GIC_REDIST);
> +if (vms->highmem_redists) {
> +virt_max_cpus += virt_redist_capacity(vms,
> VIRT_HIGH_GIC_REDIST2);
> +}
> }
>
> if (max_cpus > virt_max_cpus) {
> error_report("Number of SMP CPUs requested (%d) exceeds max CPUs "
> "supported by machine 'mach-virt' (%d)",
> max_cpus, virt_max_cpus);
> +if (vms->gic_version != VIRT_GIC_VERSION_2 && !vms->highmem_redists)
> {
> +error_printf("Try 'highmem-redists=on' for more CPUs\n");
> +}
> +
> exit(1);
> }
>
> @@ -2371,6 +2377,49 @@ static void virt_set_compact_highmem(Object *obj, bool
> value, Error **errp)
> vms->highmem_compact = value;
> }
>
> +static bool virt_get_highmem_redists(Object *obj, Error **errp)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +return vms->highmem_redists;
> +}
> +
> +static void virt_set_highmem_redists(Object *obj, bool value, Error **errp)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +vms->highmem_redists = value;
> +}
> +
> +static bool virt_get_highmem_ecam(Object *obj, Error **errp)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +return vms->highmem_ecam;
> +}
> +
> +static void virt_set_highmem_ecam(Object *obj, bool value, Error **errp)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +vms->highmem_ecam = value;
> +}
> +
> +static bool virt_get_highmem_mmio(Object *obj, Error **errp)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +return vms->highmem_mmio;
> +}
> +
> +static void virt_set_highmem_mmio(Object *obj, bool value, Error **errp)
> +{
> +VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> +vms->highmem_mmio = value;
> +}
> +
> +
> static bool virt_get_its(Object *obj, Error **errp)
> {
> VirtMachineState *vms = VIRT_MACHINE(obj);
> @@ -2996,6 +3045,28 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
>"Set on/off to enable/disable
> compact "
>"layout for high memory regions");
>
> +object_class_property_add_bool(oc, "highmem-redists",
> + virt_get_highmem_redists,
> + virt_set_highmem_redists);
> +object_class_property_set_description(oc, "highmem-redists",
> + "Set
[PATCH] tests/unit: simpler variable sequence for test-io-channel
This avoids some compilers complaining about a potentially
un-initialised [src|dst]argv. In retrospect using GString was overkill
for what we are constructing.
Signed-off-by: Alex Bennée
---
tests/unit/test-io-channel-command.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/tests/unit/test-io-channel-command.c
b/tests/unit/test-io-channel-command.c
index 43e29c8cfb..19f72eab96 100644
--- a/tests/unit/test-io-channel-command.c
+++ b/tests/unit/test-io-channel-command.c
@@ -33,19 +33,13 @@ static void test_io_channel_command_fifo(bool async)
{
g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XX",
NULL);
g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
-g_autoptr(GString) srcargs = g_string_new(socat);
-g_autoptr(GString) dstargs = g_string_new(socat);
-g_auto(GStrv) srcargv;
-g_auto(GStrv) dstargv;
+g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat,
fifo);
+g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat,
fifo);
+g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
+g_auto(GStrv) dstargv = g_strsplit(dstargs, " ", -1);
QIOChannel *src, *dst;
QIOChannelTest *test;
-g_string_append_printf(srcargs, " - PIPE:%s,wronly", fifo);
-g_string_append_printf(dstargs, " PIPE:%s,rdonly -", fifo);
-
-srcargv = g_strsplit(srcargs->str, " ", -1);
-dstargv = g_strsplit(dstargs->str, " ", -1);
-
src = QIO_CHANNEL(qio_channel_command_new_spawn((const char **) srcargv,
O_WRONLY,
&error_abort));
--
2.34.1
Re: [PATCH] tests/unit: simpler variable sequence for test-io-channel
On 3/11/22 11:23, Alex Bennée wrote: This avoids some compilers complaining about a potentially un-initialised [src|dst]argv. In retrospect using GString was overkill for what we are constructing. Signed-off-by: Alex Bennée --- tests/unit/test-io-channel-command.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH] tests/unit: simpler variable sequence for test-io-channel
Le 03/11/2022 à 11:23, Alex Bennée a écrit :
This avoids some compilers complaining about a potentially
un-initialised [src|dst]argv. In retrospect using GString was overkill
for what we are constructing.
Signed-off-by: Alex Bennée
---
tests/unit/test-io-channel-command.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/tests/unit/test-io-channel-command.c
b/tests/unit/test-io-channel-command.c
index 43e29c8cfb..19f72eab96 100644
--- a/tests/unit/test-io-channel-command.c
+++ b/tests/unit/test-io-channel-command.c
@@ -33,19 +33,13 @@ static void test_io_channel_command_fifo(bool async)
{
g_autofree gchar *tmpdir = g_dir_make_tmp("qemu-test-io-channel.XX",
NULL);
g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
-g_autoptr(GString) srcargs = g_string_new(socat);
-g_autoptr(GString) dstargs = g_string_new(socat);
-g_auto(GStrv) srcargv;
-g_auto(GStrv) dstargv;
+g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly", socat,
fifo);
+g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -", socat,
fifo);
+g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
+g_auto(GStrv) dstargv = g_strsplit(dstargs, " ", -1);
QIOChannel *src, *dst;
QIOChannelTest *test;
-g_string_append_printf(srcargs, " - PIPE:%s,wronly", fifo);
-g_string_append_printf(dstargs, " PIPE:%s,rdonly -", fifo);
-
-srcargv = g_strsplit(srcargs->str, " ", -1);
-dstargv = g_strsplit(dstargs->str, " ", -1);
-
src = QIO_CHANNEL(qio_channel_command_new_spawn((const char **) srcargv,
O_WRONLY,
&error_abort));
Reviewed-by: Laurent Vivier
Do you want this be merged via trivial branch?
Thanks,
Laurent
Re: Issue with VDUSE (QSD vduse-blk export) and vhost-vdpa
On Wed, Oct 26, 2022 at 12:17 PM Stefano Garzarella wrote: > > On Wed, Oct 26, 2022 at 05:39:23PM +0800, Yongji Xie wrote: > >Hi Stefano, > > > >On Wed, Oct 26, 2022 at 5:12 PM Stefano Garzarella > >wrote: > >> > >> Hi Xie, > >> I was testing libblkio [1] with QSD vduse-blk export and had some > >> issues. > >> > >> In a nutshell, QSD prints me the following messages when using > >> vhost-vdpa to access the device: > >> > >> Failed to get vq[0] iova mapping > >> Failed to update vring for vq[0] > >> > >> This happens only with vhost-vdpa, using virtio-vdpa instead the device > >> works fine. > >> I'm using Linux v6.0 and QEMU master (commit > >> 214a8da23651f2472b296b3293e619fd58d9e212). > >> > >> I haven't had much time to investigate, I hope to do it next week, but > >> maybe it's much faster for you. > >> > >> I saw that ioctl(VDUSE_IOTLB_GET_FD) in libvduse.c returns -1 (EPERM), > >> so IIUC in the kernel vduse_dev_broken() was called, and the device is > >> in a broken state. > >> > >> > >> We will use libblkio in QEMU [2] to access vDPA devices via vhost-vdpa. > >> But I'm doing these tests without QEMU for now, using an example inside > >> the libblkio repo: > >> > >> # Build libblkio and examples > >> # Fedora/CentOS/RHEL > >> dnf install -y git meson rust cargo python3-docutils rustfmt > >> # Debian/Ubuntu > >> apt-get install -y git meson rustc cargo python3-docutils > >> > >> git clone https://gitlab.com/libblkio/libblkio.git > >> > >> cd libblkio > >> git checkout v1.1.0 > >> > >> meson setup build > >> meson compile -C build > >> > >> > >> # On terminal 1 > >> modprobe vduse > >> modprobe vhost-vdpa > >> > >> qemu-img create -f qcow2 -o preallocation=full /path/to/test.qcow2 1g > >> > >> qemu-storage-daemon \ > >> --blockdev > >> file,filename=/path/to/test.qcow2,cache.direct=on,aio=native,node-name=file > >> \ > >> --blockdev qcow2,file=file,node-name=qcow2 \ > >> --object iothread,id=iothread0 \ > >> --export > >> vduse-blk,id=vduse0,name=vduse0,num-queues=1,node-name=qcow2,writable=on,iothread=iothread0 > >> > >> > >> # On terminal 2 > >> vdpa dev add name vduse0 mgmtdev vduse > >> > >> cd libblkio/build > >> > >> # blkio-bench executes > >> ./examples/blkio-bench virtio-blk-vhost-vdpa \ > >> path=/dev/vhost-vdpa-0 --runtime=5 --readwrite=randread > >> > >> # after this step, QSD (running on terminal 1) prints the following > >> messages: > >> Failed to get vq[0] iova mapping > >> Failed to update vring for vq[0] > >> > >> I don't know if I'm doing something wrong or in libblkio we have some > >> issue, but using vdpa-sim-blk works correctly, so maybe there is > >> something in vduse that is missing. > >> > >> Any help or suggestion is welcome :-) > >> > > > >I'd like to know whether bio-bench uses the shared memory > >(tmpfs/hugetlbfs) as the vdpa memory region. This is what VDUSE needs. > > Okay, so IIUC every memory regions should have an associated fd. > > The buffers in libblkio are already allocated in this way, but it is not > true for the virtqueue memory, I'll change it and test. I just changed the virtqueue memory allocation as you suggested, and it worked! MR here: https://gitlab.com/libblkio/libblkio/-/merge_requests/141 Thanks for the help, Stefano
[PATCH v3 0/4] compare machine type compat_props
This script is necessary to choose the best machine type in the appropriate cases. Also we have to check compat_props of the old MT after changes to be sure that they haven't broken old the MT. For example, pc_compat_3_1 of pc-q35-3.1 has Icelake-Client which was removed in March. v3 -> v2: * simplify adding new methods for getting QEMU default values * add typing * change concept from fixed dictionaries to classes v2 -> v1: * fix script code style and descriptions * reorder patches v1 -> previous iteration: * new default value print concept * QEMU python library is used to collect qmp data * remove auxiliary patches (that was used to fix `->get` sematics) * print compat_props in the correct order * delete `absract` field to reduce output JSON size Maksim Davydov (4): qom: add default value python/qmp: increase read buffer size qmp: add dump machine type compatible properties scripts: add script to compare compatible properties hw/core/machine-qmp-cmds.c| 22 +- python/qemu/qmp/qmp_client.py | 4 +- qapi/machine.json | 54 - qom/qom-qmp-cmds.c| 2 + scripts/compare_mt.py | 440 ++ tests/qtest/fuzz/qos_fuzz.c | 2 +- 6 files changed, 518 insertions(+), 6 deletions(-) create mode 100755 scripts/compare_mt.py -- 2.25.1
[PATCH v3 2/4] python/qmp: increase read buffer size
After modification of "query-machines" command the buffer size should be more than 452kB to contain output with compat-props. Signed-off-by: Maksim Davydov Reviewed-by: Vladimir Sementsov-Ogievskiy --- python/qemu/qmp/qmp_client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/qemu/qmp/qmp_client.py b/python/qemu/qmp/qmp_client.py index 5dcda04a75..659fe4d98c 100644 --- a/python/qemu/qmp/qmp_client.py +++ b/python/qemu/qmp/qmp_client.py @@ -197,8 +197,8 @@ async def run(self, address='/tmp/qemu.socket'): #: Logger object used for debugging messages. logger = logging.getLogger(__name__) -# Read buffer limit; large enough to accept query-qmp-schema -_limit = (256 * 1024) +# Read buffer limit; large enough to accept query-machines +_limit = (512 * 1024) # Type alias for pending execute() result items _PendingT = Union[Message, ExecInterruptedError] -- 2.25.1
[PATCH v3 4/4] scripts: add script to compare compatible properties
This script run QEMU to obtain compat_props of machines and default
values of different types and produce appropriate table. This table
can be used to compare machine types to choose the most suitable
machine. Also this table in json or csv format should be used to check that
new machine doesn't affect previous ones by comparing tables with and
without new machine.
Default values of properties are needed to fill "holes" in the table (one
machine has these properties and another not. For instance, 2.12 mt has
`{ "EPYC-" TYPE_X86_CPU, "xlevel", "0x800a" }`, but compat_pros of
3.1 mt doesn't have it. So, to compare these machines we need to fill
unknown value of "EPYC-x86_64-cpu-xlevel" for 3.1 mt. This unknown value
in the table I called "hole". To get values (default values) for these
"holes" the script uses list of appropriate methods.)
Notes:
* some init values from the devices can't be available like properties
from virtio-9p when configure has --disable-virtfs. This situations will
be seen in the table as "unavailable driver".
* Default values can be obtained in an unobvious way, like x86 features.
If the script doesn't know how to get property default value to compare
one machine with another it fills "holes" with "unavailable method". This
is done because script uses whitelist model to get default values of
different types. It means that the method that can't be applied to a new
type that can crash this script. It is better to get an "unavailable
driver" when creating a new machine with new compatible properties than
to break this script. So it turns out a more stable and generic script.
* If the default value can't be obtained because this property doesn't
exist or because this property can't have default value, appropriate
"hole" will be filled by "unknown property" or "no default value"
* If the property is applied to the abstract class, the script collects
default values from all child classes (set of default values)
Example:
./scripts/compare_mt.py --mt pc-q35-3.1 pc-q35-4.0
╒═══╤══╤╕
│ │ pc-q35-3.1 │
pc-q35-4.0 │
╞═══╪══╪╡
│ Cascadelake-Server-x86_64-cpu:mpx │ True │
False│
├───┼──┼┤
│ Cascadelake-Server-x86_64-cpu:stepping│ 5 │
6 │
├───┼──┼┤
│ Icelake-Client-x86_64-cpu:mpx │ True │
unavailable driver │
├───┼──┼┤
│ Icelake-Server-x86_64-cpu:mpx │ True │
False│
├───┼──┼┤
│ Opteron_G3-x86_64-cpu:rdtscp │False │
True│
├───┼──┼┤
│ Opteron_G4-x86_64-cpu:rdtscp │False │
True│
├───┼──┼┤
│ Opteron_G5-x86_64-cpu:rdtscp │False │
True│
├───┼──┼┤
│ Skylake-Client-IBRS-x86_64-cpu:mpx│ True │
False│
├───┼──┼┤
│ Skylake-Client-x86_64-cpu:mpx │ True │
False│
├───┼──┼┤
│ Skylake-Server-IBRS-x86_64-cpu:mpx│ True │
False│
├───┼──┼┤
│ Skylake-Server-x86_64-cpu:mpx │ True │
False│
├───┼──┼┤
│ intel-iommu:dma-drain │False │
True│
├───┼──┼┤
│ memory-backend-file:x-use-canonical-path-for-ramblock-id │ True │
no default value │
├───┼──┼┤
│ memory-backend-memfd:x-use-canonical-path-for-ramblock-id │ True │
no default v
[PATCH v3 1/4] qom: add default value
qmp_qom_list_properties can print default values if they are available as qmp_device_list_properties does, because both of them use the ObjectPropertyInfo structure with default_value field. This can be useful when working with "not device" types. Signed-off-by: Maksim Davydov Reviewed-by: Vladimir Sementsov-Ogievskiy --- qom/qom-qmp-cmds.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c index 2e63a4c184..1d7867dc19 100644 --- a/qom/qom-qmp-cmds.c +++ b/qom/qom-qmp-cmds.c @@ -217,6 +217,8 @@ ObjectPropertyInfoList *qmp_qom_list_properties(const char *typename, info->type = g_strdup(prop->type); info->has_description = !!prop->description; info->description = g_strdup(prop->description); +info->default_value = qobject_ref(prop->defval); +info->has_default_value = !!info->default_value; QAPI_LIST_PREPEND(prop_list, info); } -- 2.25.1
[PATCH v3 3/4] qmp: add dump machine type compatible properties
To control that creating new machine type doesn't affect the previous
types (their compat_props) and to check complex compat_props inheritance
we need qmp command to print machine type compatible properties.
This patch adds the ability to get list of all the compat_props of the
corresponding supported machines for their comparison via new optional
argument of "query-machines" command.
Signed-off-by: Maksim Davydov
Reviewed-by: Vladimir Sementsov-Ogievskiy
---
hw/core/machine-qmp-cmds.c | 22 ++-
qapi/machine.json | 54 +++--
tests/qtest/fuzz/qos_fuzz.c | 2 +-
3 files changed, 74 insertions(+), 4 deletions(-)
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 4f4ab30f8c..a6fc387439 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -74,7 +74,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp)
return head;
}
-MachineInfoList *qmp_query_machines(Error **errp)
+MachineInfoList *qmp_query_machines(bool has_compat_props, bool compat_props,
+Error **errp)
{
GSList *el, *machines = object_class_get_list(TYPE_MACHINE, false);
MachineInfoList *mach_list = NULL;
@@ -107,6 +108,25 @@ MachineInfoList *qmp_query_machines(Error **errp)
info->default_ram_id = g_strdup(mc->default_ram_id);
info->has_default_ram_id = true;
}
+if (compat_props && mc->compat_props) {
+int i;
+info->compat_props = NULL;
+CompatPropertyList **tail = &(info->compat_props);
+info->has_compat_props = true;
+
+for (i = 0; i < mc->compat_props->len; i++) {
+GlobalProperty *mt_prop = g_ptr_array_index(mc->compat_props,
+i);
+CompatProperty *prop;
+
+prop = g_malloc0(sizeof(*prop));
+prop->driver = g_strdup(mt_prop->driver);
+prop->property = g_strdup(mt_prop->property);
+prop->value = g_strdup(mt_prop->value);
+
+QAPI_LIST_APPEND(tail, prop);
+}
+}
QAPI_LIST_PREPEND(mach_list, info);
}
diff --git a/qapi/machine.json b/qapi/machine.json
index b9228a5e46..07d3b01358 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -127,6 +127,25 @@
##
{ 'command': 'query-cpus-fast', 'returns': [ 'CpuInfoFast' ] }
+##
+# @CompatProperty:
+#
+# Machine type compatible property. It's based on GlobalProperty and created
+# for machine type compat properties (see scripts)
+#
+# @driver: name of the driver that has GlobalProperty
+#
+# @property: global property name
+#
+# @value: global property value
+#
+# Since: 7.2
+##
+{ 'struct': 'CompatProperty',
+ 'data': { 'driver': 'str',
+'property': 'str',
+'value': 'str' } }
+
##
# @MachineInfo:
#
@@ -155,6 +174,9 @@
#
# @default-ram-id: the default ID of initial RAM memory backend (since 5.2)
#
+# @compat-props: List of compatible properties that defines machine type
+#(since 7.2)
+#
# Since: 1.2
##
{ 'struct': 'MachineInfo',
@@ -162,18 +184,46 @@
'*is-default': 'bool', 'cpu-max': 'int',
'hotpluggable-cpus': 'bool', 'numa-mem-supported': 'bool',
'deprecated': 'bool', '*default-cpu-type': 'str',
-'*default-ram-id': 'str' } }
+'*default-ram-id': 'str', '*compat-props': ['CompatProperty'] } }
##
# @query-machines:
#
# Return a list of supported machines
#
+# @compat-props: if true return will contain information about machine type
+#compatible properties (since 7.2)
+#
# Returns: a list of MachineInfo
#
# Since: 1.2
+#
+# Example:
+#
+# -> { "execute": "query-machines", "arguments": { "compat-props": true } }
+# <- { "return": [
+# {
+# "hotpluggable-cpus": true,
+# "name": "pc-q35-6.2",
+# "compat-props": [
+# {
+# "driver": "virtio-mem",
+# "property": "unplugged-inaccessible",
+# "value": "off"
+# }
+# ],
+# "numa-mem-supported": false,
+# "default-cpu-type": "qemu64-x86_64-cpu",
+# "cpu-max": 288,
+# "deprecated": false,
+# "default-ram-id": "pc.ram"
+# },
+# ...
+#}
##
-{ 'command': 'query-machines', 'returns': ['MachineInfo'] }
+{ 'command': 'query-machines',
+ 'data': { '*compat-props': 'bool' },
+ 'returns': ['MachineInfo'] }
##
# @CurrentMachineParams:
diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index 3a3d9c16dd..9420f950fd 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -46,7 +46,7 @@ static void qos_set_machines_devices_available(void)
MachineInfoList *mach_info;
ObjectTypeInf
Re: [PATCH 01/26] target/s390x: Use tcg_constant_* in local contexts
On Wed, Oct 05, 2022 at 08:43:56PM -0700, Richard Henderson wrote: > Replace tcg_const_* with tcg_constant_* in contexts > where the free to remove is nearby. > > Reviewed-by: Philippe Mathieu-Daudé > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 408 +-- > 1 file changed, 145 insertions(+), 263 deletions(-) Reviewed-by: Ilya Leoshkevich
[PATCH] qom.json: default the prealloc-threads to smp-cpus
Since the amount of prealloc-threads to smp-cpus is defaulted in hostmem, so sync this information. Signed-off-by: Zhenyu Zhang --- qapi/qom.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/qom.json b/qapi/qom.json index 87fcad2423..ac4cd213a7 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -576,7 +576,7 @@ # # @prealloc: if true, preallocate memory (default: false) # -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1) +# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1) # # @prealloc-context: thread context to use for creation of preallocation threads #(default: none) (since 7.2) -- 2.31.1
Re: [PATCH 02/26] target/s390x: Use tcg_constant_* for DisasCompare
On Wed, Oct 05, 2022 at 08:43:57PM -0700, Richard Henderson wrote: > The a and b fields are not modified by the consumer, > and while we need not free a constant, tcg will quietly > ignore such frees, so free_compare need not be changed. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 44 ++-- > 1 file changed, 22 insertions(+), 22 deletions(-) I did not check all the code paths in the consumer, but if there is a bug and a or b ends up being modified, one of the temp_readonly() assertions will catch it. Reviewed-by: Ilya Leoshkevich
Re: [PATCH 03/26] target/s390x: Use tcg_constant_i32 for fpinst_extract_m34
On Wed, Oct 05, 2022 at 08:43:58PM -0700, Richard Henderson wrote: > Return a constant or NULL, which means the free may be > removed from all callers of fpinst_extract_m34. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 26 +- > 1 file changed, 1 insertion(+), 25 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH] qom.json: default the prealloc-threads to smp-cpus
On 3/11/22 11:47, Zhenyu Zhang wrote: Since the amount of prealloc-threads to smp-cpus is defaulted in hostmem, so sync this information. Signed-off-by: Zhenyu Zhang --- qapi/qom.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qapi/qom.json b/qapi/qom.json index 87fcad2423..ac4cd213a7 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -576,7 +576,7 @@ # # @prealloc: if true, preallocate memory (default: false) # -# @prealloc-threads: number of CPU threads to use for prealloc (default: 1) +# @prealloc-threads: number of CPU threads to use for prealloc (default: smp-cpus) (since 7.1) The property is present since 5.0. Shouldn't this be "(default: smp-cpus) (since 5.0)"? # # @prealloc-context: thread context to use for creation of preallocation threads #(default: none) (since 7.2)
Re: [PATCH 04/26] target/s390x: Use tcg_constant_* in translate_vx.c.inc
On Wed, Oct 05, 2022 at 08:43:59PM -0700, Richard Henderson wrote:
> In most cases, this is a simple local allocate and free
> replaced by tcg_constant_*. In three cases, a variable
> temp was initialized with a constant value -- reorg to
> localize the constant.
>
> Signed-off-by: Richard Henderson
> ---
> target/s390x/tcg/translate_vx.c.inc | 45 +
> 1 file changed, 20 insertions(+), 25 deletions(-)
...
> @@ -1359,7 +1359,7 @@ static DisasJumpType op_va(DisasContext *s, DisasOps *o)
> static void gen_acc(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b, uint8_t es)
> {
> const uint8_t msb_bit_nr = NUM_VEC_ELEMENT_BITS(es) - 1;
> -TCGv_i64 msb_mask = tcg_const_i64(dup_const(es, 1ull << msb_bit_nr));
> +TCGv_i64 msb_mask = tcg_constant_i64(dup_const(es, 1ull << msb_bit_nr));
> TCGv_i64 t1 = tcg_temp_new_i64();
> TCGv_i64 t2 = tcg_temp_new_i64();
> TCGv_i64 t3 = tcg_temp_new_i64();
This also fixes a leak, right?
...
Reviewed-by: Ilya Leoshkevich
Re: [PATCH] tests/qtest/e1000e-test: Use e1000_regs.h
On 3/11/22 10:54, Akihiko Odaki wrote: The register definitions in tests/qtest/e1000e-test.c had names different from hw/net/e1000_regs.h, which made it hard to understand what test codes corresponds to the implementation. Use hw/net/e1000_regs.h from tests/qtest/libqos/e1000e.c to remove these duplications. Signed-off-by: Akihiko Odaki --- tests/qtest/e1000e-test.c | 66 ++- 1 file changed, 10 insertions(+), 56 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH v14 16/17] tests/qtest: netdev: test stream and dgram backends
On 3/11/22 10:33, Laurent Vivier wrote:
On 10/28/22 07:04, Jason Wang wrote:
在 2022/10/21 17:09, Laurent Vivier 写道:
Signed-off-by: Laurent Vivier
Acked-by: Michael S. Tsirkin
---
I got this:
63/63 ERROR:../tests/qtest/netdev-socket.c:139:test_stream_inet_ipv6:
assertion failed (resp == expect): ("st0:
index=0,type=stream,connection error\r\n" == "st0:
index=0,type=stream,tcp:::1:40389\r\n") ERROR
63/63 qemu:qtest+qtest-x86_64 /
qtest-x86_64/netdev-socket ERROR 5.29s killed by
signal 6 SIGABRT
>>> QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-x86_64
MALLOC_PERTURB_=96
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
G_TEST_DBUS_DAEMON=/home/devel/git/qemu/tests/dbus-vmstate-daemon.sh
/home/devel/git/qemu/build/tests/qtest/netdev-socket --tap -k
―――
✀
―――
stderr:
**
ERROR:../tests/qtest/netdev-socket.c:139:test_stream_inet_ipv6:
assertion failed (resp == expect): ("st0:
index=0,type=stream,connection error\r\n" == "st0:
index=0,type=stream,tcp:::1:40389\r\n")
(test program exited with status code -6)
I'm not able to reproduce the problem.
Is this 100% reproducible?
Is IPv6 enabled on your test machine?
If IPv6 is not available, this test should be skipped, not failing.
Re: [PATCH] tests/qtest/libqos/e1000e: Use E1000_STATUS_ASDV_1000
On 3/11/22 09:34, Akihiko Odaki wrote: Nemonics E1000_STATUS_LAN_INIT_DONE and E1000_STATUS_ASDV_1000 have the same value, and E1000_STATUS_ASDV_1000 should be used here because E1000_STATUS_ASDV_1000 represents the auto-detected speed tested here while E1000_STATUS_LAN_INIT_DONE is a value used for a different purpose with a variant of e1000e family different from the one implemented in QEMU. Signed-off-by: Akihiko Odaki --- tests/qtest/libqos/e1000e.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 05/26] target/s390x: Change help_goto_direct to work on displacements
On Wed, Oct 05, 2022 at 08:44:00PM -0700, Richard Henderson wrote: > In preparation for TARGET_TB_PCREL, reduce reliance on absolute values. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH 08/26] target/s390x: Use gen_psw_addr_disp in pc_to_link_info
On Wed, Oct 05, 2022 at 08:44:03PM -0700, Richard Henderson wrote: > This is slightly more complicated that a straight displacement > for 31 and 24-bit modes. Dont bother with a cant-happen assert. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 21 - > 1 file changed, 12 insertions(+), 9 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH v14 16/17] tests/qtest: netdev: test stream and dgram backends
On 11/3/22 12:07, Philippe Mathieu-Daudé wrote:
On 3/11/22 10:33, Laurent Vivier wrote:
On 10/28/22 07:04, Jason Wang wrote:
在 2022/10/21 17:09, Laurent Vivier 写道:
Signed-off-by: Laurent Vivier
Acked-by: Michael S. Tsirkin
---
I got this:
63/63 ERROR:../tests/qtest/netdev-socket.c:139:test_stream_inet_ipv6: assertion failed
(resp == expect): ("st0: index=0,type=stream,connection error\r\n" == "st0:
index=0,type=stream,tcp:::1:40389\r\n") ERROR
63/63 qemu:qtest+qtest-x86_64 / qtest-x86_64/netdev-socket ERROR 5.29s
killed by signal 6 SIGABRT
>>> QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-x86_64
MALLOC_PERTURB_=96
QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon
G_TEST_DBUS_DAEMON=/home/devel/git/qemu/tests/dbus-vmstate-daemon.sh
/home/devel/git/qemu/build/tests/qtest/netdev-socket --tap -k
――― ✀
―――
stderr:
**
ERROR:../tests/qtest/netdev-socket.c:139:test_stream_inet_ipv6: assertion failed (resp
== expect): ("st0: index=0,type=stream,connection error\r\n" == "st0:
index=0,type=stream,tcp:::1:40389\r\n")
(test program exited with status code -6)
I'm not able to reproduce the problem.
Is this 100% reproducible?
Is IPv6 enabled on your test machine?
If IPv6 is not available, this test should be skipped, not failing.
I agree.
But I'm not sure it's the real cause of the problem (perhaps a firewall
problem?).
I think I should update my inet_get_free_port_socket() to get a free port from AF_INET6
and not only from AF_INET.
Thanks,
Laurent
Re: [PATCH] tests/unit: simpler variable sequence for test-io-channel
Laurent Vivier writes:
> Le 03/11/2022 à 11:23, Alex Bennée a écrit :
>> This avoids some compilers complaining about a potentially
>> un-initialised [src|dst]argv. In retrospect using GString was overkill
>> for what we are constructing.
>> Signed-off-by: Alex Bennée
>> ---
>> tests/unit/test-io-channel-command.c | 14 --
>> 1 file changed, 4 insertions(+), 10 deletions(-)
>> diff --git a/tests/unit/test-io-channel-command.c
>> b/tests/unit/test-io-channel-command.c
>> index 43e29c8cfb..19f72eab96 100644
>> --- a/tests/unit/test-io-channel-command.c
>> +++ b/tests/unit/test-io-channel-command.c
>> @@ -33,19 +33,13 @@ static void test_io_channel_command_fifo(bool async)
>> {
>> g_autofree gchar *tmpdir =
>> g_dir_make_tmp("qemu-test-io-channel.XX", NULL);
>> g_autofree gchar *fifo = g_strdup_printf("%s/%s", tmpdir, TEST_FIFO);
>> -g_autoptr(GString) srcargs = g_string_new(socat);
>> -g_autoptr(GString) dstargs = g_string_new(socat);
>> -g_auto(GStrv) srcargv;
>> -g_auto(GStrv) dstargv;
>> +g_autofree gchar *srcargs = g_strdup_printf("%s - PIPE:%s,wronly",
>> socat, fifo);
>> +g_autofree gchar *dstargs = g_strdup_printf("%s PIPE:%s,rdonly -",
>> socat, fifo);
>> +g_auto(GStrv) srcargv = g_strsplit(srcargs, " ", -1);
>> +g_auto(GStrv) dstargv = g_strsplit(dstargs, " ", -1);
>> QIOChannel *src, *dst;
>> QIOChannelTest *test;
>> -g_string_append_printf(srcargs, " - PIPE:%s,wronly", fifo);
>> -g_string_append_printf(dstargs, " PIPE:%s,rdonly -", fifo);
>> -
>> -srcargv = g_strsplit(srcargs->str, " ", -1);
>> -dstargv = g_strsplit(dstargs->str, " ", -1);
>> -
>> src = QIO_CHANNEL(qio_channel_command_new_spawn((const char **)
>> srcargv,
>> O_WRONLY,
>> &error_abort));
>
> Reviewed-by: Laurent Vivier
>
> Do you want this be merged via trivial branch?
I'm easy either way. I've got a for-7.2/misc-fixes branch which I'll
send once I can figure out whats going on with the avocado console
interaction code.
>
> Thanks,
> Laurent
--
Alex Bennée
Re: [PATCH 06/26] target/s390x: Introduce gen_psw_addr_disp
On Wed, Oct 05, 2022 at 08:44:01PM -0700, Richard Henderson wrote: > In preparation for TARGET_TB_PCREL, reduce reliance on absolute values. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 69 > 1 file changed, 46 insertions(+), 23 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH 07/26] target/s390x: Remove pc argument to pc_to_link_into
On Wed, Oct 05, 2022 at 08:44:02PM -0700, Richard Henderson wrote: > All callers pass s->pc_tmp. > > Reviewed-by: Philippe Mathieu-Daudé > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH] Add missing include statement for global xml_builtin
On 3/11/22 09:38, Stefan Weil via wrote: This fixes some compiler warnings with compiler flag -Wmissing-variable-declarations (tested with clang): aarch64_be-linux-user-gdbstub-xml.c:564:19: warning: no previous extern declaration for non-static variable 'xml_builtin' [-Wmissing-variable-declarations] aarch64-linux-user-gdbstub-xml.c:564:19: warning: no previous extern declaration for non-static variable 'xml_builtin' [-Wmissing-variable-declarations] aarch64-softmmu-gdbstub-xml.c:1763:19: warning: no previous extern declaration for non-static variable 'xml_builtin' [-Wmissing-variable-declarations] Signed-off-by: Stefan Weil --- scripts/feature_to_c.sh | 1 + 1 file changed, 1 insertion(+) Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH for 7.2] Fix broken configure with -Wunused-parameter
On Wed, 2 Nov 2022 at 20:24, Stefan Weil via wrote:
>
> The configure script fails because it tries to compile small C programs
> with a main function which is declared with arguments argc and argv
> although those arguments are unused.
>
> Running `configure -extra-cflags=-Wunused-parameter` triggers the problem.
> configure for a native build does abort but shows the error in config.log.
> A cross build configure for Windows with Debian stable aborts with an
> error.
>
> Avoiding unused arguments fixes this.
>
> Signed-off-by: Stefan Weil
> ---
>
> See https://gitlab.com/qemu-project/qemu/-/issues/1295.
>
> I noticed the problem because I often compile with -Wextra.
>
> Stefan
>
> configure | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/configure b/configure
> index 4275f5419f..1106c04fea 100755
> --- a/configure
> +++ b/configure
> @@ -1258,6 +1258,7 @@ if test "$stack_protector" != "no"; then
>cat > $TMPC << EOF
> int main(int argc, char *argv[])
> {
> +(void)argc;
I'm not a huge fan of this syntax, and it doesn't match the way
we deal with "argument is unused" elsewhere in the codebase
(where we either don't care about it or else use the GCC 'unused'
attribute hidden behind the glib G_GNUC_UNUSED macro).
I am surprised that this didn't get caught by the check in
do_compiler_werror(), which is supposed to report "this
configure test passed without -Werror but failed with
-Werror, so configure is probably buggy.". That's what's
supposed to catch "your compiler warns on stuff our doesn't
in the test case programs".
If you're building with --disable-werror then configure
should be OK anyway. This is probably a good idea if you want
to build with extra warning arguments in --extra-cflags.
If it doesn't work right even with --disable-werror that's
also something we should investigate.
> char arr[64], *p = arr, *c = argv[0];
> while (*c) {
> *p++ = *c++;
> @@ -1607,7 +1608,7 @@ fi
>
> if test "$safe_stack" = "yes"; then
> cat > $TMPC << EOF
> -int main(int argc, char *argv[])
> +int main(void)
> {
> #if ! __has_feature(safe_stack)
> #error SafeStack Disabled
> @@ -1629,7 +1630,7 @@ EOF
>fi
> else
> cat > $TMPC << EOF
> -int main(int argc, char *argv[])
> +int main(void)
> {
> #if defined(__has_feature)
> #if __has_feature(safe_stack)
> @@ -1675,7 +1676,7 @@ static const int Z = 1;
> #define TAUT(X) ((X) == Z)
> #define PAREN(X, Y) (X == Y)
> #define ID(X) (X)
> -int main(int argc, char *argv[])
> +int main(void)
> {
> int x = 0, y = 0;
> x = ID(x);
No objection to the cases where we can pass "void", that's
a neater way to write the test anyway.
thanks
-- PMM
Re: [PATCH v3] target/i386: Fix calculation of LOCK NEG eflags
ping
On 2022/10/24 16:41, Qi Hu wrote:
In sequence:
---
lock negl -0x14(%rbp)
pushf
pop%rax
---
%rax will obtain the wrong value becasue the "lock neg" calculates the
wrong eflags. The "s->T0" is updated by the wrong value.
You can use this to do some test:
---
#include
int main()
{
__volatile__ unsigned test = 0x2363a;
__volatile__ char cond = 0;
asm(
"lock negl %0 \n\t"
"sets %1"
: "=m"(test), "=r"(cond)
:
:);
assert(cond & 1);
return 0;
}
---
Reported-by: Jinyang Shen
Co-Developed-by: Xuehai Chen
Signed-off-by: Xuehai Chen
Signed-off-by: Qi Hu
---
V2 -> V3:
Fix typo "caculation".
V1 -> V2:
Following Richard's suggestion, just change mov to neg instead of using
local_tmp.
---
target/i386/tcg/translate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index e19d5c1c64..cec2182080 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -3299,7 +3299,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
tcg_temp_free(t2);
tcg_temp_free(a0);
-tcg_gen_mov_tl(s->T0, t0);
+tcg_gen_neg_tl(s->T0, t0);
tcg_temp_free(t0);
} else {
tcg_gen_neg_tl(s->T0, s->T0);
Re: [PATCH for 7.2] Fix broken configure with -Wunused-parameter
Am 03.11.22 um 12:48 schrieb Peter Maydell:
On Wed, 2 Nov 2022 at 20:24, Stefan Weil via wrote:
The configure script fails because it tries to compile small C programs
with a main function which is declared with arguments argc and argv
although those arguments are unused.
Running `configure -extra-cflags=-Wunused-parameter` triggers the problem.
configure for a native build does abort but shows the error in config.log.
A cross build configure for Windows with Debian stable aborts with an
error.
Avoiding unused arguments fixes this.
Signed-off-by: Stefan Weil
---
See https://gitlab.com/qemu-project/qemu/-/issues/1295.
I noticed the problem because I often compile with -Wextra.
Stefan
configure | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/configure b/configure
index 4275f5419f..1106c04fea 100755
--- a/configure
+++ b/configure
@@ -1258,6 +1258,7 @@ if test "$stack_protector" != "no"; then
cat > $TMPC << EOF
int main(int argc, char *argv[])
{
+(void)argc;
I'm not a huge fan of this syntax, and it doesn't match the way
we deal with "argument is unused" elsewhere in the codebase
(where we either don't care about it or else use the GCC 'unused'
attribute hidden behind the glib G_GNUC_UNUSED macro).
Any other variant is also fine for me, for example "using" argc by a
"return argc == 0;" instead of "return 0;". Would that be better? If
there is an accepted variant, I can either send a v2 patch, or maybe
such a trivial change can be applied when merging.
I am surprised that this didn't get caught by the check in
do_compiler_werror(), which is supposed to report "this
configure test passed without -Werror but failed with
-Werror, so configure is probably buggy.". That's what's
supposed to catch "your compiler warns on stuff our doesn't
in the test case programs".
If you're building with --disable-werror then configure
should be OK anyway. This is probably a good idea if you want
to build with extra warning arguments in --extra-cflags.
If it doesn't work right even with --disable-werror that's
also something we should investigate.
Cross builds for Windows fail with and without --disable-werror. See
also my bug report https://gitlab.com/qemu-project/qemu/-/issues/1295.
You are right that this is strange and should be investigated,
especially because native builds don't fail like that.
char arr[64], *p = arr, *c = argv[0];
while (*c) {
*p++ = *c++;
@@ -1607,7 +1608,7 @@ fi
if test "$safe_stack" = "yes"; then
cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
{
#if ! __has_feature(safe_stack)
#error SafeStack Disabled
@@ -1629,7 +1630,7 @@ EOF
fi
else
cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
{
#if defined(__has_feature)
#if __has_feature(safe_stack)
@@ -1675,7 +1676,7 @@ static const int Z = 1;
#define TAUT(X) ((X) == Z)
#define PAREN(X, Y) (X == Y)
#define ID(X) (X)
-int main(int argc, char *argv[])
+int main(void)
{
int x = 0, y = 0;
x = ID(x);
No objection to the cases where we can pass "void", that's
a neater way to write the test anyway.
thanks
-- PMM
OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v3 3/4] hw/nvme: add iothread support
On Nov 3 09:51, Jinhao Fan wrote: > at 7:13 PM, Klaus Jensen wrote: > > > Otherwise, it all looks fine. I'm still seeing the weird slowdown when > > an iothread is enabled. I have yet to figure out why that is... But it > > scales! :) > > How much slowdown do you observe on your machine? > I'll rerun some experiments and get back to you :) signature.asc Description: PGP signature
Re: [PATCH v3 4/4] hw/nvme: add polling support
On Nov 3 10:18, Jinhao Fan wrote: > at 7:10 PM, Klaus Jensen wrote: > > > This doesn't do what you expect it to. By not updaring the eventidx it > > will fall behind the actual head, causing the host to think that the > > device is not processing events (but it is!), resulting in doorbell > > ringing. > > I’m not sure I understand this correctly. > > In 7.13.1 in NVMe Spec 1.4c it says "If updating an entry in the Shadow > Doorbell buffer **changes** the value from being less than or equal to the > value of the corresponding EventIdx buffer entry to being greater than that > value, then the host shall also update the controller's corresponding > doorbell register to match the value of that entry in the Shadow Doorbell > buffer.” > > So my understanding is that once the eventidx falls behind the actual head, > the host will only ring the doorbell once but *not* for future submissions. > > Is this not what real hosts are doing? I agree that the spec is a little unclear on this point. In any case, in Linux, when the driver has decided that the sq tail must be updated, it will use this check: (new_idx - event_idx - 1) < (new_idx - old) So it doesn't account for if or not eventidx was already behind. signature.asc Description: PGP signature
Re: [PULL v2 00/82] pci,pc,virtio: features, tests, fixes, cleanups
On Wed, Nov 02, 2022 at 03:47:43PM -0400, Stefan Hajnoczi wrote: > On Wed, Nov 02, 2022 at 12:02:14PM -0400, Michael S. Tsirkin wrote: > > Changes from v1: > > > > Applied and squashed fixes by Igor, Lei He, Hesham Almatary for > > bugs that tripped up the pipeline. > > Updated expected files for core-count test. > > Several "make check" CI failures have occurred. They look like they are > related. Here is one (see the URLs at the bottom of this email for more > details): > > 17/106 ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child > process > (/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch/subprocess > [8609]) failed unexpectedly ERROR > 17/106 qemu:qtest+qtest-arm / qtest-arm/qos-test ERROR >31.44s killed by signal 6 SIGABRT > >>> G_TEST_DBUS_DAEMON=/builds/qemu-project/qemu/tests/dbus-vmstate-daemon.sh > >>> MALLOC_PERTURB_=49 QTEST_QEMU_IMG=./qemu-img > >>> QTEST_QEMU_BINARY=./qemu-system-arm > >>> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon > >>> /builds/qemu-project/qemu/build/tests/qtest/qos-test --tap -k > ― ✀ ― > stderr: > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) > qemu-system-arm: Failed to set msg fds. > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) > qemu-system-arm: -chardev > socket,id=chr-reconnect,path=/tmp/vhost-test-6PT2U1/reconnect.sock,server=on: > info: QEMU waiting for connection on: > disconnected:unix:/tmp/vhost-test-6PT2U1/reconnect.sock,server=on > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) > qemu-system-arm: Failed to set msg fds. > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) > qemu-system-arm: -chardev > socket,id=chr-connect-fail,path=/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on: > info: QEMU waiting for connection on: > disconnected:unix:/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on > qemu-system-arm: -netdev > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: Failed to read msg > header. Read 0 instead of 12. Original request 1. > qemu-system-arm: -netdev > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: vhost_backend_init > failed: Protocol error > qemu-system-arm: -netdev > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: failed to init > vhost_net for queue 0 > qemu-system-arm: -netdev > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: info: QEMU waiting > for connection on: > disconnected:unix:/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) > qemu-system-arm: Failed to set msg fds. > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) > qemu-system-arm: -chardev > socket,id=chr-flags-mismatch,path=/tmp/vhost-test-94UYU1/flags-mismatch.sock,server=on: > info: QEMU waiting for connection on: > disconnected:unix:/tmp/vhost-test-94UYU1/flags-mismatch.sock,server=on > qemu-system-arm: Failed to write msg. Wrote -1 instead of 52. > qemu-system-arm: vhost_set_mem_table failed: Invalid argument (22) > qemu-system-arm: Failed to set msg fds. > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) > UndefinedBehaviorSanitizer:DEADLYSIGNAL > ==8618==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address > 0x (pc 0x55e34deccab0 bp 0x sp 0x7ffc94894710 T8618) > ==8618==The signal is caused by a READ memory access. > ==8618==Hint: address points to the zero page. > #0 0x55e34deccab0 in ldl_he_p > /builds/qemu-project/qemu/include/qemu/bswap.h:301:5 > #1 0x55e34deccab0 in ldn_he_p > /builds/qemu-project/qemu/include/qemu/bswap.h:440:1 > #2 0x55e34deccab0 in flatview_write_continue > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2824:19 > #3 0x55e34dec9f21 in flatview_write > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2867:12 > #4 0x55e34dec9f21 in address_space_write > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2963:18 > #5 0x55e34decace7 in address_space_unmap > /builds/qemu-project/qemu/build/../softmmu/physmem.c:3306:9 > #6 0x55e34de6d4ec in vhost_memory_unmap > /builds/qemu-project/qemu/build/../hw/virtio/vhost.c:342:9 > #7 0x55e34de6d4ec in vhost_virtqueue_stop > /builds/qemu-project/qemu/build/../hw/virtio/vhost.c:1242:5 > #8 0x55e34de72904 in vhost_dev_stop > /builds/qemu-project/qemu/build/../hw/virtio/vhost.c:1882:9 > #9 0x55e34d890514 in vhost_net_stop_one > /builds/qemu-project/qemu/build/../hw/net/vhost_net.c:331:5 > #10 0x55e34d88fef6 in vhost_net_
Re: [PATCH v9 0/8] KVM: mm: fd-based approach for supporting KVM
On Tue, Oct 25, 2022 at 8:48 PM Chao Peng wrote: > > This patch series implements KVM guest private memory for confidential > computing scenarios like Intel TDX[1]. If a TDX host accesses > TDX-protected guest memory, machine check can happen which can further > crash the running host system, this is terrible for multi-tenant > configurations. The host accesses include those from KVM userspace like > QEMU. This series addresses KVM userspace induced crash by introducing > new mm and KVM interfaces so KVM userspace can still manage guest memory > via a fd-based approach, but it can never access the guest memory > content. > > The patch series touches both core mm and KVM code. I appreciate > Andrew/Hugh and Paolo/Sean can review and pick these patches. Any other > reviews are always welcome. > - 01: mm change, target for mm tree > - 02-08: KVM change, target for KVM tree > > Given KVM is the only current user for the mm part, I have chatted with > Paolo and he is OK to merge the mm change through KVM tree, but > reviewed-by/acked-by is still expected from the mm people. > > The patches have been verified in Intel TDX environment, but Vishal has > done an excellent work on the selftests[4] which are dedicated for this > series, making it possible to test this series without innovative > hardware and fancy steps of building a VM environment. See Test section > below for more info. > > > Introduction > > KVM userspace being able to crash the host is horrible. Under current > KVM architecture, all guest memory is inherently accessible from KVM > userspace and is exposed to the mentioned crash issue. The goal of this > series is to provide a solution to align mm and KVM, on a userspace > inaccessible approach of exposing guest memory. > > Normally, KVM populates secondary page table (e.g. EPT) by using a host > virtual address (hva) from core mm page table (e.g. x86 userspace page > table). This requires guest memory being mmaped into KVM userspace, but > this is also the source where the mentioned crash issue can happen. In > theory, apart from those 'shared' memory for device emulation etc, guest > memory doesn't have to be mmaped into KVM userspace. > > This series introduces fd-based guest memory which will not be mmaped > into KVM userspace. KVM populates secondary page table by using a With no mappings in place for userspace VMM, IIUC, looks like the host kernel will not be able to find the culprit userspace process in case of Machine check error on guest private memory. As implemented in hwpoison_user_mappings, host kernel tries to look at the processes which have mapped the pfns with hardware error. Is there a modification needed in mce handling logic of the host kernel to immediately send a signal to the vcpu thread accessing faulting pfn backing guest private memory? > fd/offset pair backed by a memory file system. The fd can be created > from a supported memory filesystem like tmpfs/hugetlbfs and KVM can > directly interact with them with newly introduced in-kernel interface, > therefore remove the KVM userspace from the path of accessing/mmaping > the guest memory. > > Kirill had a patch [2] to address the same issue in a different way. It > tracks guest encrypted memory at the 'struct page' level and relies on > HWPOISON to reject the userspace access. The patch has been discussed in > several online and offline threads and resulted in a design document [3] > which is also the original proposal for this series. Later this patch > series evolved as more comments received in community but the major > concepts in [3] still hold true so recommend reading. > > The patch series may also be useful for other usages, for example, pure > software approach may use it to harden itself against unintentional > access to guest memory. This series is designed with these usages in > mind but doesn't have code directly support them and extension might be > needed. > > > mm change > = > Introduces a new memfd_restricted system call which can create memory > file that is restricted from userspace access via normal MMU operations > like read(), write() or mmap() etc and the only way to use it is > passing it to a third kernel module like KVM and relying on it to > access the fd through the newly added restrictedmem kernel interface. > The restrictedmem interface bridges the memory file subsystems > (tmpfs/hugetlbfs etc) and their users (KVM in this case) and provides > bi-directional communication between them. > > > KVM change > == > Extends the KVM memslot to provide guest private (encrypted) memory from > a fd. With this extension, a single memslot can maintain both private > memory through private fd (restricted_fd/restricted_offset) and shared > (unencrypted) memory through userspace mmaped host virtual address > (userspace_addr). For a particular guest page, the corresponding page in > KVM memslot can be only either private or shared and only one of the > shared/private parts
Re: [PATCH v4 0/6] Only generate cluster node in PPTT when specified
On 2022/11/2 16:17, Michael S. Tsirkin wrote: > On Tue, Nov 01, 2022 at 03:10:42PM +0800, Yicong Yang wrote: >> From: Yicong Yang >> >> This series mainly change the policy for building a cluster topology node >> in PPTT. Previously we'll always build a cluster node in PPTT without >> asking the user, after this set the cluster node will be built only the >> the user specify through "-smp clusters=X". >> >> One problem is related to this but not fully caused by this, see the >> discussion in [*]. When booting the VM with `-smp 8` and 4 numa nodes, >> the linux scheduling domains in the VM misses the NUMA domains. It's >> because the MC level span extends to Cluster level (which is generated >> by the Qemu by default) that spans all the cpus in the system, then the >> scheduling domain building stops at MC level since it already includes all >> the cpus. >> >> Considering cluster is an optional level and most platforms don't have it, >> they may even don't realize this is built and a always build policy cannot >> emulate the real topology on these platforms. So in this series improve the >> policy to only generate cluster when the user explicitly want it. >> >> Update the tests and test tables accordingly. > > I think we can classify this as a bugfix and so allow after Not quite sure about it as I regarded it as an improvement of the topology building policy. And the problem I met is not directly caused by the policy before this series. > the freeze, however, this needs ack from ARM maintainers then. sure. Will resend after the freeze. Thanks. > > >> [*] >> https://lore.kernel.org/lkml/2c079860-ee82-7719-d3d2-756192f41...@huawei.com/ >> >> Change since v3: >> - Improve and attach the diff of the affected ACPI tables in the commit, and >> minor cleanups >> Link: >> https://lore.kernel.org/qemu-devel/20221031090523.34146-1-yangyic...@huawei.com/ >> >> Change since v2: >> - Add tag from Micheal, thanks >> - Handle the tests changes with bios-tables-test-allowed-diff.h, Per Micheal >> - Address the comments per Yanan >> Link: >> https://lore.kernel.org/qemu-devel/20221027032613.18377-1-yangyic...@huawei.com/ >> >> Change since v1: >> - Only includes the test tables which is really needed >> - Enrich the commit >> Link: >> https://lore.kernel.org/qemu-devel/20220922131143.58003-1-yangyic...@huawei.com/ >> >> Yicong Yang (6): >> tests: virt: Allow changes to PPTT test table >> hw/acpi/aml-build: Only generate cluster node in PPTT when specified >> tests: virt: Update expected ACPI tables for virt test >> tests: acpi: Add and whitelist *.topology blobs >> tests: acpi: aarch64: Add topology test for aarch64 >> tests: acpi: aarch64: Add *.topology tables >> >> hw/acpi/aml-build.c| 2 +- >> hw/core/machine-smp.c | 2 ++ >> include/hw/boards.h| 3 +++ >> qemu-options.hx| 3 +++ >> tests/data/acpi/virt/APIC.topology | Bin 0 -> 700 bytes >> tests/data/acpi/virt/DSDT.topology | Bin 0 -> 5398 bytes >> tests/data/acpi/virt/PPTT | Bin 96 -> 76 bytes >> tests/data/acpi/virt/PPTT.topology | Bin 0 -> 336 bytes >> tests/qtest/bios-tables-test.c | 19 +++ >> 9 files changed, 28 insertions(+), 1 deletion(-) >> create mode 100644 tests/data/acpi/virt/APIC.topology >> create mode 100644 tests/data/acpi/virt/DSDT.topology >> create mode 100644 tests/data/acpi/virt/PPTT.topology >> >> -- >> 2.24.0 > > . >
[PATCH v2] target/loongarch: Fix emulation of float-point disable exception
We need to emulate it to generate a floating point disable exception
when CSR.EUEN.FPE is zero.
Signed-off-by: Rui Wang
---
target/loongarch/cpu.c| 2 ++
target/loongarch/cpu.h| 13 +++
.../loongarch/insn_trans/trans_farith.c.inc | 30
target/loongarch/insn_trans/trans_fcmp.c.inc | 11 --
.../loongarch/insn_trans/trans_fmemory.c.inc | 34 +++
target/loongarch/insn_trans/trans_fmov.c.inc | 29 ++--
.../insn_trans/trans_privileged.c.inc | 2 +-
target/loongarch/translate.c | 2 +-
8 files changed, 110 insertions(+), 13 deletions(-)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 1512664214..46b04cbdad 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -48,6 +48,7 @@ static const char * const excp_names[] = {
[EXCCODE_BRK] = "Break",
[EXCCODE_INE] = "Instruction Non-Existent",
[EXCCODE_IPE] = "Instruction privilege error",
+[EXCCODE_FPD] = "Floating Point Disabled",
[EXCCODE_FPE] = "Floating Point Exception",
[EXCCODE_DBP] = "Debug breakpoint",
[EXCCODE_BCE] = "Bound Check Exception",
@@ -185,6 +186,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
case EXCCODE_BRK:
case EXCCODE_INE:
case EXCCODE_IPE:
+case EXCCODE_FPD:
case EXCCODE_FPE:
case EXCCODE_BCE:
env->CSR_BADV = env->pc;
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index dbce176564..2729a114c2 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -14,6 +14,7 @@
#include "qemu/timer.h"
#include "exec/memory.h"
#include "hw/sysbus.h"
+#include "cpu-csr.h"
#define IOCSRF_TEMP 0
#define IOCSRF_NODECNT 1
@@ -391,6 +392,14 @@ static inline int cpu_mmu_index(CPULoongArchState *env,
bool ifetch)
#endif
}
+/*
+ * LoongArch CPUs hardware flags.
+ * bit[2..0] for MMU index.
+ * bit[7..4] for CSR.EUEN.{ BTE, ASXE, SXE, FPE }.
+ */
+#define HW_FLAGS_MMU_MASK 0x07
+#define HW_FLAGS_EUEN_FPE 0x10
+
static inline void cpu_get_tb_cpu_state(CPULoongArchState *env,
target_ulong *pc,
target_ulong *cs_base,
@@ -399,6 +408,10 @@ static inline void cpu_get_tb_cpu_state(CPULoongArchState
*env,
*pc = env->pc;
*cs_base = 0;
*flags = cpu_mmu_index(env, false);
+
+if (FIELD_EX64(env->CSR_EUEN, CSR_EUEN, FPE)) {
+*flags |= HW_FLAGS_EUEN_FPE;
+}
}
void loongarch_cpu_list(void);
diff --git a/target/loongarch/insn_trans/trans_farith.c.inc
b/target/loongarch/insn_trans/trans_farith.c.inc
index 7bb3f41aee..e2dec75dfb 100644
--- a/target/loongarch/insn_trans/trans_farith.c.inc
+++ b/target/loongarch/insn_trans/trans_farith.c.inc
@@ -3,9 +3,22 @@
* Copyright (c) 2021 Loongson Technology Corporation Limited
*/
+#ifndef CONFIG_USER_ONLY
+#define CHECK_FPE do { \
+if ((ctx->base.tb->flags & HW_FLAGS_EUEN_FPE) == 0) { \
+generate_exception(ctx, EXCCODE_FPD); \
+return false; \
+} \
+} while (0)
+#else
+#define CHECK_FPE
+#endif
+
static bool gen_fff(DisasContext *ctx, arg_fff *a,
void (*func)(TCGv, TCGv_env, TCGv, TCGv))
{
+CHECK_FPE;
+
func(cpu_fpr[a->fd], cpu_env, cpu_fpr[a->fj], cpu_fpr[a->fk]);
return true;
}
@@ -13,6 +26,8 @@ static bool gen_fff(DisasContext *ctx, arg_fff *a,
static bool gen_ff(DisasContext *ctx, arg_ff *a,
void (*func)(TCGv, TCGv_env, TCGv))
{
+CHECK_FPE;
+
func(cpu_fpr[a->fd], cpu_env, cpu_fpr[a->fj]);
return true;
}
@@ -22,6 +37,9 @@ static bool gen_muladd(DisasContext *ctx, arg_ *a,
int flag)
{
TCGv_i32 tflag = tcg_constant_i32(flag);
+
+CHECK_FPE;
+
func(cpu_fpr[a->fd], cpu_env, cpu_fpr[a->fj],
cpu_fpr[a->fk], cpu_fpr[a->fa], tflag);
return true;
@@ -29,18 +47,24 @@ static bool gen_muladd(DisasContext *ctx, arg_ *a,
static bool trans_fcopysign_s(DisasContext *ctx, arg_fcopysign_s *a)
{
+CHECK_FPE;
+
tcg_gen_deposit_i64(cpu_fpr[a->fd], cpu_fpr[a->fk], cpu_fpr[a->fj], 0, 31);
return true;
}
static bool trans_fcopysign_d(DisasContext *ctx, arg_fcopysign_d *a)
{
+CHECK_FPE;
+
tcg_gen_deposit_i64(cpu_fpr[a->fd], cpu_fpr[a->fk], cpu_fpr[a->fj], 0, 63);
return true;
}
static bool trans_fabs_s(DisasContext *ctx, arg_fabs_s *a)
{
+CHECK_FPE;
+
tcg_gen_andi_i64(cpu_fpr[a->fd], cpu_fpr[a->fj], MAKE_64BIT_MASK(0, 31));
gen_nanbox_s(cpu_fpr[a->fd], cpu_fpr[a->fd]);
return true;
@@ -48,12 +72,16 @@ static bool trans_fabs_s(DisasContext *ctx, arg_fabs_s *a)
static bool trans_fabs_d(DisasContext *ctx, arg_fabs_d *a)
{
+CHECK_FPE;
+
tcg_gen_andi_i64(cpu_fpr[a->fd], cpu_fpr[a->fj], MAKE_64BIT_MASK(0, 63));
return true;
}
static bool trans_fneg_s(DisasContext *ctx, arg_fneg_s *a)
{
+CHECK_FPE;
+
[PULL 7/7] target/loongarch: Fix raise_mmu_exception() set wrong exception_index
When the address is invalid address, We should set exception_index
according to MMUAccessType, and EXCCODE_ADEF need't update badinstr.
Otherwise, The system enters an infinite loop. e.g:
run test.c on system mode
test.c:
#include
void (*func)(int *);
int main()
{
int i = 8;
void *ptr = (void *)0x4000;
func = ptr;
func(&i);
return 0;
}
Signed-off-by: Song Gao
Reviewed-by: Richard Henderson
Message-ID: <20221101073210.3934280-2-gaos...@loongson.cn>
---
target/loongarch/cpu.c| 1 +
target/loongarch/tlb_helper.c | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index b28aaed5ba..1512664214 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -177,6 +177,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
}
QEMU_FALLTHROUGH;
case EXCCODE_PIF:
+case EXCCODE_ADEF:
cause = cs->exception_index;
update_badinstr = 0;
break;
diff --git a/target/loongarch/tlb_helper.c b/target/loongarch/tlb_helper.c
index 610b6d123c..d2f8fb0c60 100644
--- a/target/loongarch/tlb_helper.c
+++ b/target/loongarch/tlb_helper.c
@@ -229,7 +229,8 @@ static void raise_mmu_exception(CPULoongArchState *env,
target_ulong address,
switch (tlb_error) {
default:
case TLBRET_BADADDR:
-cs->exception_index = EXCCODE_ADEM;
+cs->exception_index = access_type == MMU_INST_FETCH
+ ? EXCCODE_ADEF : EXCCODE_ADEM;
break;
case TLBRET_NOMATCH:
/* No TLB match for a mapped address */
@@ -643,7 +644,7 @@ bool loongarch_cpu_tlb_fill(CPUState *cs, vaddr address,
int size,
CPULoongArchState *env = &cpu->env;
hwaddr physical;
int prot;
-int ret = TLBRET_BADADDR;
+int ret;
/* Data access */
ret = get_physical_address(env, &physical, &prot, address,
--
2.31.1
[PULL 0/7] loongarch-to-apply queue
The following changes since commit a11f65ec1b8adcb012b89c92819cbda4dc25aaf1: Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2022-11-01 13:49:33 -0400) are available in the Git repository at: https://gitlab.com/gaosong/qemu.git tags/pull-loongarch-20221103 for you to fetch changes up to d31e2b1af7e6db41e6088679babc3893bd69b4b3: target/loongarch: Fix raise_mmu_exception() set wrong exception_index (2022-11-03 17:59:19 +0800) pull-loongarch-20221103 Song Gao (2): target/loongarch: Add exception subcode target/loongarch: Fix raise_mmu_exception() set wrong exception_index Xiaojuan Yang (5): hw/intc: Convert the memops to with_attrs in LoongArch extioi hw/intc: Fix LoongArch extioi coreisr accessing hw/loongarch: Load FDT table into dram memory space hw/loongarch: Improve fdt for LoongArch virt machine hw/loongarch: Add TPM device for LoongArch virt machine hw/intc/loongarch_extioi.c | 41 - hw/intc/trace-events| 3 +-- hw/loongarch/acpi-build.c | 50 +-- hw/loongarch/virt.c | 53 - include/hw/loongarch/virt.h | 3 --- include/hw/pci-host/ls7a.h | 1 + target/loongarch/cpu.c | 8 -- target/loongarch/cpu.h | 58 ++--- target/loongarch/iocsr_helper.c | 19 -- target/loongarch/tlb_helper.c | 5 ++-- 10 files changed, 170 insertions(+), 71 deletions(-)
[PULL 3/7] hw/loongarch: Load FDT table into dram memory space
From: Xiaojuan Yang
Load FDT table into dram memory space, and the addr is 2 MiB.
Since lowmem region starts from 0, FDT base address is located
at 2 MiB to avoid NULL pointer access.
Signed-off-by: Xiaojuan Yang
Acked-by: Song Gao
Message-Id: <20221028014007.2718352-2-yangxiaoj...@loongson.cn>
Signed-off-by: Song Gao
---
hw/loongarch/virt.c | 18 +++---
include/hw/loongarch/virt.h | 3 ---
2 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 4b595a9ea4..50e9829a94 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -159,7 +159,6 @@ static void fdt_add_pcie_node(const LoongArchMachineState
*lams)
1, FDT_PCI_RANGE_MMIO, 2, base_mmio,
2, base_mmio, 2, size_mmio);
g_free(nodename);
-qemu_fdt_dumpdtb(ms->fdt, lams->fdt_size);
}
static void fdt_add_irqchip_node(LoongArchMachineState *lams)
@@ -656,6 +655,7 @@ static void loongarch_init(MachineState *machine)
MemoryRegion *address_space_mem = get_system_memory();
LoongArchMachineState *lams = LOONGARCH_MACHINE(machine);
int i;
+hwaddr fdt_base;
if (!cpu_model) {
cpu_model = LOONGARCH_CPU_TYPE_NAME("la464");
@@ -760,12 +760,16 @@ static void loongarch_init(MachineState *machine)
lams->machine_done.notify = virt_machine_done;
qemu_add_machine_init_done_notifier(&lams->machine_done);
fdt_add_pcie_node(lams);
-
-/* load fdt */
-MemoryRegion *fdt_rom = g_new(MemoryRegion, 1);
-memory_region_init_rom(fdt_rom, NULL, "fdt", VIRT_FDT_SIZE, &error_fatal);
-memory_region_add_subregion(get_system_memory(), VIRT_FDT_BASE, fdt_rom);
-rom_add_blob_fixed("fdt", machine->fdt, lams->fdt_size, VIRT_FDT_BASE);
+/*
+ * Since lowmem region starts from 0, FDT base address is located
+ * at 2 MiB to avoid NULL pointer access.
+ *
+ * Put the FDT into the memory map as a ROM image: this will ensure
+ * the FDT is copied again upon reset, even if addr points into RAM.
+ */
+fdt_base = 2 * MiB;
+qemu_fdt_dumpdtb(machine->fdt, lams->fdt_size);
+rom_add_blob_fixed("fdt", machine->fdt, lams->fdt_size, fdt_base);
}
bool loongarch_is_acpi_enabled(LoongArchMachineState *lams)
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 09f1c88ee5..45c383f5a7 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -28,9 +28,6 @@
#define VIRT_GED_MEM_ADDR (VIRT_GED_EVT_ADDR + ACPI_GED_EVT_SEL_LEN)
#define VIRT_GED_REG_ADDR (VIRT_GED_MEM_ADDR + MEMORY_HOTPLUG_IO_LEN)
-#define VIRT_FDT_BASE 0x1c40
-#define VIRT_FDT_SIZE 0x10
-
struct LoongArchMachineState {
/*< private >*/
MachineState parent_obj;
--
2.31.1
[PULL 1/7] hw/intc: Convert the memops to with_attrs in LoongArch extioi
From: Xiaojuan Yang
Converting the MemoryRegionOps read/write handlers to
with_attrs in LoongArch extioi emulation.
Signed-off-by: Xiaojuan Yang
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Message-Id: <20221021015307.2570844-2-yangxiaoj...@loongson.cn>
Signed-off-by: Song Gao
---
hw/intc/loongarch_extioi.c | 31 +--
hw/intc/trace-events | 3 +--
2 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
index 22803969bc..72f4b0cde5 100644
--- a/hw/intc/loongarch_extioi.c
+++ b/hw/intc/loongarch_extioi.c
@@ -68,44 +68,45 @@ static void extioi_setirq(void *opaque, int irq, int level)
extioi_update_irq(s, irq, level);
}
-static uint64_t extioi_readw(void *opaque, hwaddr addr, unsigned size)
+static MemTxResult extioi_readw(void *opaque, hwaddr addr, uint64_t *data,
+unsigned size, MemTxAttrs attrs)
{
LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
unsigned long offset = addr & 0x;
-uint32_t index, cpu, ret = 0;
+uint32_t index, cpu;
switch (offset) {
case EXTIOI_NODETYPE_START ... EXTIOI_NODETYPE_END - 1:
index = (offset - EXTIOI_NODETYPE_START) >> 2;
-ret = s->nodetype[index];
+*data = s->nodetype[index];
break;
case EXTIOI_IPMAP_START ... EXTIOI_IPMAP_END - 1:
index = (offset - EXTIOI_IPMAP_START) >> 2;
-ret = s->ipmap[index];
+*data = s->ipmap[index];
break;
case EXTIOI_ENABLE_START ... EXTIOI_ENABLE_END - 1:
index = (offset - EXTIOI_ENABLE_START) >> 2;
-ret = s->enable[index];
+*data = s->enable[index];
break;
case EXTIOI_BOUNCE_START ... EXTIOI_BOUNCE_END - 1:
index = (offset - EXTIOI_BOUNCE_START) >> 2;
-ret = s->bounce[index];
+*data = s->bounce[index];
break;
case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
-ret = s->coreisr[cpu][index];
+*data = s->coreisr[cpu][index];
break;
case EXTIOI_COREMAP_START ... EXTIOI_COREMAP_END - 1:
index = (offset - EXTIOI_COREMAP_START) >> 2;
-ret = s->coremap[index];
+*data = s->coremap[index];
break;
default:
break;
}
-trace_loongarch_extioi_readw(addr, ret);
-return ret;
+trace_loongarch_extioi_readw(addr, *data);
+return MEMTX_OK;
}
static inline void extioi_enable_irq(LoongArchExtIOI *s, int index,\
@@ -127,8 +128,9 @@ static inline void extioi_enable_irq(LoongArchExtIOI *s,
int index,\
}
}
-static void extioi_writew(void *opaque, hwaddr addr,
- uint64_t val, unsigned size)
+static MemTxResult extioi_writew(void *opaque, hwaddr addr,
+ uint64_t val, unsigned size,
+ MemTxAttrs attrs)
{
LoongArchExtIOI *s = LOONGARCH_EXTIOI(opaque);
int i, cpu, index, old_data, irq;
@@ -231,11 +233,12 @@ static void extioi_writew(void *opaque, hwaddr addr,
default:
break;
}
+return MEMTX_OK;
}
static const MemoryRegionOps extioi_ops = {
-.read = extioi_readw,
-.write = extioi_writew,
+.read_with_attrs = extioi_readw,
+.write_with_attrs = extioi_writew,
.impl.min_access_size = 4,
.impl.max_access_size = 4,
.valid.min_access_size = 4,
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 0a90c1cdec..6fbc2045e6 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -306,6 +306,5 @@ loongarch_msi_set_irq(int irq_num) "set msi irq %d"
# loongarch_extioi.c
loongarch_extioi_setirq(int irq, int level) "set extirq irq %d level %d"
-loongarch_extioi_readw(uint64_t addr, uint32_t val) "addr: 0x%"PRIx64 "val:
0x%x"
+loongarch_extioi_readw(uint64_t addr, uint64_t val) "addr: 0x%"PRIx64 "val:
0x%" PRIx64
loongarch_extioi_writew(uint64_t addr, uint64_t val) "addr: 0x%"PRIx64 "val:
0x%" PRIx64
-
--
2.31.1
[PULL 4/7] hw/loongarch: Improve fdt for LoongArch virt machine
From: Xiaojuan Yang
Add new items into LoongArch FDT, including rtc and uart info.
Signed-off-by: Xiaojuan Yang
Reviewed-by: Song Gao
Message-Id: <20221028014007.2718352-3-yangxiaoj...@loongson.cn>
Signed-off-by: Song Gao
---
hw/loongarch/virt.c| 31 +++
include/hw/pci-host/ls7a.h | 1 +
2 files changed, 32 insertions(+)
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 50e9829a94..afc1c8ac77 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -42,6 +42,35 @@
#include "hw/display/ramfb.h"
#include "hw/mem/pc-dimm.h"
+static void fdt_add_rtc_node(LoongArchMachineState *lams)
+{
+char *nodename;
+hwaddr base = VIRT_RTC_REG_BASE;
+hwaddr size = VIRT_RTC_LEN;
+MachineState *ms = MACHINE(lams);
+
+nodename = g_strdup_printf("/rtc@%" PRIx64, base);
+qemu_fdt_add_subnode(ms->fdt, nodename);
+qemu_fdt_setprop_string(ms->fdt, nodename, "compatible",
"loongson,ls7a-rtc");
+qemu_fdt_setprop_sized_cells(ms->fdt, nodename, "reg", 0x0, base, size);
+g_free(nodename);
+}
+
+static void fdt_add_uart_node(LoongArchMachineState *lams)
+{
+char *nodename;
+hwaddr base = VIRT_UART_BASE;
+hwaddr size = VIRT_UART_SIZE;
+MachineState *ms = MACHINE(lams);
+
+nodename = g_strdup_printf("/serial@%" PRIx64, base);
+qemu_fdt_add_subnode(ms->fdt, nodename);
+qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
+qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
+qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 1);
+g_free(nodename);
+}
+
static void create_fdt(LoongArchMachineState *lams)
{
MachineState *ms = MACHINE(lams);
@@ -422,6 +451,7 @@ static void loongarch_devices_init(DeviceState *pch_pic,
LoongArchMachineState *
qdev_get_gpio_in(pch_pic,
VIRT_UART_IRQ - PCH_PIC_IRQ_OFFSET),
115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
+fdt_add_uart_node(lams);
/* Network init */
for (i = 0; i < nb_nics; i++) {
@@ -442,6 +472,7 @@ static void loongarch_devices_init(DeviceState *pch_pic,
LoongArchMachineState *
sysbus_create_simple("ls7a_rtc", VIRT_RTC_REG_BASE,
qdev_get_gpio_in(pch_pic,
VIRT_RTC_IRQ - PCH_PIC_IRQ_OFFSET));
+fdt_add_rtc_node(lams);
pm_mem = g_new(MemoryRegion, 1);
memory_region_init_io(pm_mem, NULL, &loongarch_virt_pm_ops,
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
index 9bd875ca8b..df7fa55a30 100644
--- a/include/hw/pci-host/ls7a.h
+++ b/include/hw/pci-host/ls7a.h
@@ -37,6 +37,7 @@
#define VIRT_PCI_IRQS48
#define VIRT_UART_IRQ(PCH_PIC_IRQ_OFFSET + 2)
#define VIRT_UART_BASE 0x1fe001e0
+#define VIRT_UART_SIZE 0X100
#define VIRT_RTC_IRQ (PCH_PIC_IRQ_OFFSET + 3)
#define VIRT_MISC_REG_BASE (VIRT_PCH_REG_BASE + 0x0008)
#define VIRT_RTC_REG_BASE(VIRT_MISC_REG_BASE + 0x00050100)
--
2.31.1
[PULL 6/7] target/loongarch: Add exception subcode
We need subcodes to distinguish the same excode cs->exception_indexs,
such as EXCCODE_ADEF/EXCCODE_ADEM.
Signed-off-by: Song Gao
Reviewed-by: Richard Henderson
Message-ID: <20221101073210.3934280-1-gaos...@loongson.cn>
---
target/loongarch/cpu.c | 7 +++--
target/loongarch/cpu.h | 58 ++
2 files changed, 36 insertions(+), 29 deletions(-)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 49393d95d8..b28aaed5ba 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -220,7 +220,10 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
env->CSR_TLBRERA = FIELD_DP64(env->CSR_TLBRERA, CSR_TLBRERA,
PC, (env->pc >> 2));
} else {
-env->CSR_ESTAT = FIELD_DP64(env->CSR_ESTAT, CSR_ESTAT, ECODE, cause);
+env->CSR_ESTAT = FIELD_DP64(env->CSR_ESTAT, CSR_ESTAT, ECODE,
+EXCODE_MCODE(cause));
+env->CSR_ESTAT = FIELD_DP64(env->CSR_ESTAT, CSR_ESTAT, ESUBCODE,
+EXCODE_SUBCODE(cause));
env->CSR_PRMD = FIELD_DP64(env->CSR_PRMD, CSR_PRMD, PPLV,
FIELD_EX64(env->CSR_CRMD, CSR_CRMD, PLV));
env->CSR_PRMD = FIELD_DP64(env->CSR_PRMD, CSR_PRMD, PIE,
@@ -257,7 +260,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
env->pc = env->CSR_TLBRENTRY;
} else {
env->pc = env->CSR_EENTRY;
-env->pc += cause * vec_size;
+env->pc += EXCODE_MCODE(cause) * vec_size;
}
qemu_log_mask(CPU_LOG_INT,
"%s: PC " TARGET_FMT_lx " ERA " TARGET_FMT_lx
diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h
index dce999aaac..dbce176564 100644
--- a/target/loongarch/cpu.h
+++ b/target/loongarch/cpu.h
@@ -75,33 +75,37 @@ FIELD(FCSR0, CAUSE, 24, 5)
#define FP_DIV0 8
#define FP_INVALID16
-#define EXCCODE_EXTERNAL_INT 64 /* plus external interrupt number */
-#define EXCCODE_INT 0
-#define EXCCODE_PIL 1
-#define EXCCODE_PIS 2
-#define EXCCODE_PIF 3
-#define EXCCODE_PME 4
-#define EXCCODE_PNR 5
-#define EXCCODE_PNX 6
-#define EXCCODE_PPI 7
-#define EXCCODE_ADEF8 /* Different exception subcode */
-#define EXCCODE_ADEM8
-#define EXCCODE_ALE 9
-#define EXCCODE_BCE 10
-#define EXCCODE_SYS 11
-#define EXCCODE_BRK 12
-#define EXCCODE_INE 13
-#define EXCCODE_IPE 14
-#define EXCCODE_FPD 15
-#define EXCCODE_SXD 16
-#define EXCCODE_ASXD17
-#define EXCCODE_FPE 18 /* Different exception subcode */
-#define EXCCODE_VFPE18
-#define EXCCODE_WPEF19 /* Different exception subcode */
-#define EXCCODE_WPEM19
-#define EXCCODE_BTD 20
-#define EXCCODE_BTE 21
-#define EXCCODE_DBP 26 /* Reserved subcode used for debug */
+#define EXCODE(code, subcode) ( ((subcode) << 6) | (code) )
+#define EXCODE_MCODE(code)( (code) & 0x3f )
+#define EXCODE_SUBCODE(code) ( (code) >> 6 )
+
+#define EXCCODE_EXTERNAL_INT64 /* plus external interrupt number */
+#define EXCCODE_INT EXCODE(0, 0)
+#define EXCCODE_PIL EXCODE(1, 0)
+#define EXCCODE_PIS EXCODE(2, 0)
+#define EXCCODE_PIF EXCODE(3, 0)
+#define EXCCODE_PME EXCODE(4, 0)
+#define EXCCODE_PNR EXCODE(5, 0)
+#define EXCCODE_PNX EXCODE(6, 0)
+#define EXCCODE_PPI EXCODE(7, 0)
+#define EXCCODE_ADEFEXCODE(8, 0) /* Different exception
subcode */
+#define EXCCODE_ADEMEXCODE(8, 1)
+#define EXCCODE_ALE EXCODE(9, 0)
+#define EXCCODE_BCE EXCODE(10, 0)
+#define EXCCODE_SYS EXCODE(11, 0)
+#define EXCCODE_BRK EXCODE(12, 0)
+#define EXCCODE_INE EXCODE(13, 0)
+#define EXCCODE_IPE EXCODE(14, 0)
+#define EXCCODE_FPD EXCODE(15, 0)
+#define EXCCODE_SXD EXCODE(16, 0)
+#define EXCCODE_ASXDEXCODE(17, 0)
+#define EXCCODE_FPE EXCODE(18, 0) /* Different exception
subcode */
+#define EXCCODE_VFPEEXCODE(18, 1)
+#define EXCCODE_WPEFEXCODE(19, 0) /* Different exception
subcode */
+#define EXCCODE_WPEMEXCODE(19, 1)
+#define EXCCODE_BTD EXCODE(20, 0)
+#define EXCCODE_BTE EXCODE(21, 0)
+#define EXCCODE_DBP EXCODE(26, 0) /* Reserved subcode used
for debug */
/* cpucfg[0] bits */
FIELD(CPUCFG0,
[PULL 5/7] hw/loongarch: Add TPM device for LoongArch virt machine
From: Xiaojuan Yang
Add TPM device for LoongArch virt machine, including
establish TPM acpi info and add TYPE_TPM_TIS_SYSBUS
to dynamic_sysbus_devices list.
Signed-off-by: Xiaojuan Yang
Reviewed-by: Song Gao
Message-Id: <20221028014007.2718352-4-yangxiaoj...@loongson.cn>
Signed-off-by: Song Gao
---
hw/loongarch/acpi-build.c | 50 +--
hw/loongarch/virt.c | 4
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 378a6d9d38..1d0e562435 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -31,6 +31,9 @@
#include "hw/acpi/generic_event_device.h"
#include "hw/pci-host/gpex.h"
+#include "sysemu/tpm.h"
+#include "hw/platform-bus.h"
+#include "hw/acpi/aml-build.h"
#define ACPI_BUILD_ALIGN_SIZE 0x1000
#define ACPI_BUILD_TABLE_SIZE 0x2
@@ -275,6 +278,41 @@ static void build_pci_device_aml(Aml *scope,
LoongArchMachineState *lams)
acpi_dsdt_add_gpex(scope, &cfg);
}
+#ifdef CONFIG_TPM
+static void acpi_dsdt_add_tpm(Aml *scope, LoongArchMachineState *vms)
+{
+PlatformBusDevice *pbus = PLATFORM_BUS_DEVICE(vms->platform_bus_dev);
+hwaddr pbus_base = VIRT_PLATFORM_BUS_BASEADDRESS;
+SysBusDevice *sbdev = SYS_BUS_DEVICE(tpm_find());
+MemoryRegion *sbdev_mr;
+hwaddr tpm_base;
+
+if (!sbdev) {
+return;
+}
+
+tpm_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
+assert(tpm_base != -1);
+
+tpm_base += pbus_base;
+
+sbdev_mr = sysbus_mmio_get_region(sbdev, 0);
+
+Aml *dev = aml_device("TPM0");
+aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
+aml_append(dev, aml_name_decl("_STR", aml_string("TPM 2.0 Device")));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+Aml *crs = aml_resource_template();
+aml_append(crs,
+ aml_memory32_fixed(tpm_base,
+ (uint32_t)memory_region_size(sbdev_mr),
+ AML_READ_WRITE));
+aml_append(dev, aml_name_decl("_CRS", crs));
+aml_append(scope, dev);
+}
+#endif
+
/* build DSDT */
static void
build_dsdt(GArray *table_data, BIOSLinker *linker, MachineState *machine)
@@ -289,7 +327,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
MachineState *machine)
build_uart_device_aml(dsdt);
build_pci_device_aml(dsdt, lams);
build_la_ged_aml(dsdt, machine);
-
+#ifdef CONFIG_TPM
+acpi_dsdt_add_tpm(dsdt, lams);
+#endif
/* System State Package */
scope = aml_scope("\\");
pkg = aml_package(4);
@@ -358,7 +398,13 @@ static void acpi_build(AcpiBuildTables *tables,
MachineState *machine)
build_mcfg(tables_blob, tables->linker, &mcfg, lams->oem_id,
lams->oem_table_id);
}
-
+/* TPM info */
+if (tpm_get_version(tpm_find()) == TPM_VERSION_2_0) {
+acpi_add_table(table_offsets, tables_blob);
+build_tpm2(tables_blob, tables->linker,
+ tables->tcpalog, lams->oem_id,
+ lams->oem_table_id);
+}
/* Add tables supplied by user (if any) */
for (u = acpi_table_first(); u; u = acpi_table_next(u)) {
unsigned len = acpi_table_len(u);
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index afc1c8ac77..5e4c2790bf 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -41,6 +41,7 @@
#include "hw/platform-bus.h"
#include "hw/display/ramfb.h"
#include "hw/mem/pc-dimm.h"
+#include "sysemu/tpm.h"
static void fdt_add_rtc_node(LoongArchMachineState *lams)
{
@@ -960,6 +961,9 @@ static void loongarch_class_init(ObjectClass *oc, void
*data)
object_class_property_set_description(oc, "acpi",
"Enable ACPI");
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
+#ifdef CONFIG_TPM
+machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
+#endif
}
static const TypeInfo loongarch_machine_types[] = {
--
2.31.1
[PULL 2/7] hw/intc: Fix LoongArch extioi coreisr accessing
From: Xiaojuan Yang
1. When cpu read or write extioi COREISR reg, it should access
the reg belonged to itself, so the cpu index of 's->coreisr'
is current cpu number. Using MemTxAttrs' requester_id to get
the cpu index.
2. it need not to mask 0x1f when calculate the coreisr array index.
Signed-off-by: Xiaojuan Yang
Reviewed-by: Richard Henderson
Message-Id: <20221021015307.2570844-3-yangxiaoj...@loongson.cn>
Signed-off-by: Song Gao
---
hw/intc/loongarch_extioi.c | 10 ++
target/loongarch/iocsr_helper.c | 19 +++
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/hw/intc/loongarch_extioi.c b/hw/intc/loongarch_extioi.c
index 72f4b0cde5..4b8ec3f28a 100644
--- a/hw/intc/loongarch_extioi.c
+++ b/hw/intc/loongarch_extioi.c
@@ -93,8 +93,9 @@ static MemTxResult extioi_readw(void *opaque, hwaddr addr,
uint64_t *data,
*data = s->bounce[index];
break;
case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
-index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
-cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
+index = (offset - EXTIOI_COREISR_START) >> 2;
+/* using attrs to get current cpu index */
+cpu = attrs.requester_id;
*data = s->coreisr[cpu][index];
break;
case EXTIOI_COREMAP_START ... EXTIOI_COREMAP_END - 1:
@@ -185,8 +186,9 @@ static MemTxResult extioi_writew(void *opaque, hwaddr addr,
s->bounce[index] = val;
break;
case EXTIOI_COREISR_START ... EXTIOI_COREISR_END - 1:
-index = ((offset - EXTIOI_COREISR_START) & 0x1f) >> 2;
-cpu = ((offset - EXTIOI_COREISR_START) >> 8) & 0x3;
+index = (offset - EXTIOI_COREISR_START) >> 2;
+/* using attrs to get current cpu index */
+cpu = attrs.requester_id;
old_data = s->coreisr[cpu][index];
s->coreisr[cpu][index] = old_data & ~val;
/* write 1 to clear interrrupt */
diff --git a/target/loongarch/iocsr_helper.c b/target/loongarch/iocsr_helper.c
index 0e9c537dc7..505853e17b 100644
--- a/target/loongarch/iocsr_helper.c
+++ b/target/loongarch/iocsr_helper.c
@@ -14,54 +14,57 @@
#include "exec/cpu_ldst.h"
#include "tcg/tcg-ldst.h"
+#define GET_MEMTXATTRS(cas) \
+((MemTxAttrs){.requester_id = env_cpu(cas)->cpu_index})
+
uint64_t helper_iocsrrd_b(CPULoongArchState *env, target_ulong r_addr)
{
return address_space_ldub(&env->address_space_iocsr, r_addr,
- MEMTXATTRS_UNSPECIFIED, NULL);
+ GET_MEMTXATTRS(env), NULL);
}
uint64_t helper_iocsrrd_h(CPULoongArchState *env, target_ulong r_addr)
{
return address_space_lduw(&env->address_space_iocsr, r_addr,
- MEMTXATTRS_UNSPECIFIED, NULL);
+ GET_MEMTXATTRS(env), NULL);
}
uint64_t helper_iocsrrd_w(CPULoongArchState *env, target_ulong r_addr)
{
return address_space_ldl(&env->address_space_iocsr, r_addr,
- MEMTXATTRS_UNSPECIFIED, NULL);
+ GET_MEMTXATTRS(env), NULL);
}
uint64_t helper_iocsrrd_d(CPULoongArchState *env, target_ulong r_addr)
{
return address_space_ldq(&env->address_space_iocsr, r_addr,
- MEMTXATTRS_UNSPECIFIED, NULL);
+ GET_MEMTXATTRS(env), NULL);
}
void helper_iocsrwr_b(CPULoongArchState *env, target_ulong w_addr,
target_ulong val)
{
address_space_stb(&env->address_space_iocsr, w_addr,
- val, MEMTXATTRS_UNSPECIFIED, NULL);
+ val, GET_MEMTXATTRS(env), NULL);
}
void helper_iocsrwr_h(CPULoongArchState *env, target_ulong w_addr,
target_ulong val)
{
address_space_stw(&env->address_space_iocsr, w_addr,
- val, MEMTXATTRS_UNSPECIFIED, NULL);
+ val, GET_MEMTXATTRS(env), NULL);
}
void helper_iocsrwr_w(CPULoongArchState *env, target_ulong w_addr,
target_ulong val)
{
address_space_stl(&env->address_space_iocsr, w_addr,
- val, MEMTXATTRS_UNSPECIFIED, NULL);
+ val, GET_MEMTXATTRS(env), NULL);
}
void helper_iocsrwr_d(CPULoongArchState *env, target_ulong w_addr,
target_ulong val)
{
address_space_stq(&env->address_space_iocsr, w_addr,
- val, MEMTXATTRS_UNSPECIFIED, NULL);
+ val, GET_MEMTXATTRS(env), NULL);
}
--
2.31.1
Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
On Wed, 2 Nov 2022, Daniel Henrique Barboza wrote:
On 11/1/22 19:29, Philippe Mathieu-Daudé wrote:
This is a respin of Bernhard's v4 with Freescale eSDHC implemented
as an 'UNIMP' region. See v4 cover here:
https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shen...@gmail.com/
Since v5:
- Rebased (ppc-next merged)
- Properly handle big-endian
Since v4:
- Do not rename ESDHC_* definitions to USDHC_*
- Do not modify SDHCIState structure
Supersedes: <20221031115402.91912-1-phi...@linaro.org>
Queued in gitlab.com/danielhb/qemu/tree/ppc-8.0 (since we missed the
freeze for 7.2).
Could you please always use ppc-next to queue patches for the next
upcoming version and ppc-7.2 for the current version? Unless this makes
your workflow harder in which case ignore this but the reason I ask is
because then it's enough for me to only track ppc-next if I need to rebase
patches on that and don't have to add a new branch at every release
(unless I have some patches to rebase on it during a freeze but that's
less likely than rebasing on your queued patches for the next release xo
using version for the current branch and keep next for the future versions
makes more sense to me).
BTW, checkpatch complained about this line being too long (83 chars):
3/3 Checking commit bc7b8cc88560 (hw/ppc/e500: Add Freescale eSDHC to
e500plat)
WARNING: line over 80 characters
#150: FILE: hw/ppc/e500.c:1024:
+pmc->ccsrbar_base +
MPC85XX_ESDHC_REGS_OFFSET,
The code except is this:
if (pmc->has_esdhc) {
create_unimplemented_device("esdhc",
pmc->ccsrbar_base +
MPC85XX_ESDHC_REGS_OFFSET,
MPC85XX_ESDHC_REGS_SIZE);
To get rid of the warning we would need to make a python-esque identation
(line
break after "(" ) or create a new variable to hold the sum. Both seems
overkill
so I'll ignore the warning. Phil is welcome to re-send if he thinks it's
worth
it.
Or you could break indentation and not start at the ( but 3 chars back. I.e.:
create_unimplemented_device("esdhc",
pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
MPC85XX_ESDHC_REGS_SIZE);
But I think it can be just ignored in this case.
And I'll follow it up with my usual plea in these cases: can we move the line
size warning to 100 chars? For QEMU 8.0? Pretty please?
I think the consensus was to keep 80 columns if possible, this is good
becuase you can open more files side by side (although it does not match
well with the long _ naming convention of glib and qemu) but we have a
distinction between checkpatch warning and error in line length. I think
it will give error at 90 chars but as long as it's just warns that means:
fix it if you can but in rare cases if it's more readable with a slightly
longer line then it is still acceptable. I think that's the case here,
splitting the line would be less readable than a few chars longer line.
Regards,
BALATON Zoltan
Re: [PATCH 09/26] target/s390x: Use gen_psw_addr_disp in save_link_info
On Wed, Oct 05, 2022 at 08:44:04PM -0700, Richard Henderson wrote: > Trivial but non-mechanical conversion away from pc_tmp. > > Reviewed-by: Philippe Mathieu-Daudé > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) I was a bit worried about this sequence and wrote a regression test. It did not find any issues, but I will post it here, might prove useful some day. Reviewed-by: Ilya Leoshkevich
[PATCH] tests/tcg/s390x: Add bal.S
Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich --- tests/tcg/s390x/Makefile.softmmu-target | 1 + tests/tcg/s390x/bal.S | 24 2 files changed, 25 insertions(+) create mode 100644 tests/tcg/s390x/bal.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index a34fa68473e..295df084919 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -7,3 +7,4 @@ QEMU_OPTS=-action panic=exit-failure -kernel -Wl,--build-id=none $< -o $@ TESTS += unaligned-lowcore +TESTS += bal diff --git a/tests/tcg/s390x/bal.S b/tests/tcg/s390x/bal.S new file mode 100644 index 000..e54d8874ff9 --- /dev/null +++ b/tests/tcg/s390x/bal.S @@ -0,0 +1,24 @@ +.org 0x200 /* lowcore padding */ +.globl _start +_start: +lpswe start24_psw +_start24: +lgrl %r0,initial_r0 +lgrl %r1,expected_r0 +bal %r0,0f +0: +cgrjne %r0,%r1,1f +lpswe success_psw +1: +lpswe failure_psw +.align 8 +start24_psw: +.quad 0x1600,_start24 /* 24-bit mode, cc = 1, pm = 6 */ +initial_r0: +.quad 0x1234567887654321 +expected_r0: +.quad 0x123456789600 + 0b /* ilc = 2, cc = 1, pm = 6 */ +success_psw: +.quad 0x2,0xfff/* see is_special_wait_psw() */ +failure_psw: +.quad 0x2,0/* disabled wait */ -- 2.37.2
Re: [PATCH 10/26] target/s390x: Use gen_psw_addr_disp in op_sam
On Wed, Oct 05, 2022 at 08:44:05PM -0700, Richard Henderson wrote: > Complicated because we may now require a runtime jump. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 40 +--- > 1 file changed, 28 insertions(+), 12 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH v3 3/4] hw/nvme: add iothread support
On 11/3/2022 8:11 PM, Klaus Jensen wrote: I'll rerun some experiments and get back to you 😄 Thanks Klaus. I'll also run some experments on my machine, to see if the reenabling cqe batching patch solves this problem.
Re: [PATCH 11/26] target/s390x: Use ilen instead in branches
On Wed, Oct 05, 2022 at 08:44:06PM -0700, Richard Henderson wrote: > Remove the remaining uses of pc_tmp, and remove the variable. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 13 +++-- > 1 file changed, 3 insertions(+), 10 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH v3] target/arm: honor HCR_E2H and HCR_TGE in ats_write64()
On Wed, 2 Nov 2022 at 05:33, Richard Henderson wrote: > > On 11/1/22 17:42, Ake Koomsin wrote: > > We need to check HCR_E2H and HCR_TGE to select the right MMU index for > > the correct translation regime. > > > > To check for EL2&0 translation regime: > > - For S1E0*, S1E1* and S12E* ops, check both HCR_E2H and HCR_TGE > > - For S1E2* ops, check only HCR_E2H > > > > Signed-off-by: Ake Koomsin > > --- > > > > v3: > > - Avoid recomputing arm_hcr_el2_eff() as recommended by Richard H. > > - Use ':?' for more compact code as recommended by Richard H. > > > > v2: > > - Rebase with the latest upstream > > - It turns out that we need to check both HCR_E2H and HCR_TGE for > >S1E0*, S1E1* and S12E* address translation as well according to the > >Architecture Manual. > > -https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg06084.html > > > > v1: > > https://lists.gnu.org/archive/html/qemu-devel/2022-10/msg02627.html > > > > target/arm/helper.c | 15 +-- > > 1 file changed, 9 insertions(+), 6 deletions(-) > > Reviewed-by: Richard Henderson Applied to target-arm.next, thanks. -- PMM
Re: [PATCH v3 4/4] hw/nvme: add polling support
On 11/3/2022 8:10 PM, Klaus Jensen wrote: I agree that the spec is a little unclear on this point. In any case, in Linux, when the driver has decided that the sq tail must be updated, it will use this check: (new_idx - event_idx - 1) < (new_idx - old) When eventidx is already behind, it's like: 0 1 <- event_idx 2 <- old 3 <- new_idx 4 . . . In this case, (new_idx - event_idx - 1) = 3-1-1 = 1 >= (new_idx - old) = 3-2=1, so the host won't update sq tail. Where am I wrong in this example? So it doesn't account for if or not eventidx was already behind.
Re: [PATCH] target/arm: Two fixes for secure ptw
On Wed, 2 Nov 2022 at 05:47, Richard Henderson
wrote:
>
> Reversed the sense of non-secure in get_phys_addr_lpae,
> and failed to initialize attrs.secure for ARMMMUIdx_Phys_S.
>
> Fixes: 48da29e4 ("target/arm: Add ptw_idx to S1Translate")
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1293
> Signed-off-by: Richard Henderson
> ---
> target/arm/ptw.c | 15 ---
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 58a7bbda50..df3573f150 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -1357,7 +1357,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> S1Translate *ptw,
> descaddr |= (address >> (stride * (4 - level))) & indexmask;
> descaddr &= ~7ULL;
> nstable = extract32(tableattrs, 4, 1);
> -if (!nstable) {
> +if (nstable) {
> /*
> * Stage2_S -> Stage2 or Phys_S -> Phys_NS
> * Assert that the non-secure idx are even, and relative order.
> @@ -2671,6 +2671,13 @@ static bool get_phys_addr_with_struct(CPUARMState
> *env, S1Translate *ptw,
> bool is_secure = ptw->in_secure;
> ARMMMUIdx s1_mmu_idx;
>
> +/*
> + * The page table entries may downgrade secure to non-secure, but
> + * cannot upgrade an non-secure translation regime's attributes
> + * to secure.
> + */
> +result->f.attrs.secure = is_secure;
> +
> switch (mmu_idx) {
> case ARMMMUIdx_Phys_S:
> case ARMMMUIdx_Phys_NS:
> @@ -2712,12 +2719,6 @@ static bool get_phys_addr_with_struct(CPUARMState
> *env, S1Translate *ptw,
> break;
> }
>
> -/*
> - * The page table entries may downgrade secure to non-secure, but
> - * cannot upgrade an non-secure translation regime's attributes
> - * to secure.
> - */
> -result->f.attrs.secure = is_secure;
> result->f.attrs.user = regime_is_user(env, mmu_idx);
Do we also need to move this setting of attrs.user ?
get_phys_addr_disabled() doesn't set that either.
thanks
-- PMM
Re: [PATCH] target/arm: Two fixes for secure ptw
On Thu, 3 Nov 2022 at 13:19, Peter Maydell wrote:
>
> On Wed, 2 Nov 2022 at 05:47, Richard Henderson
> wrote:
> >
> > Reversed the sense of non-secure in get_phys_addr_lpae,
> > and failed to initialize attrs.secure for ARMMMUIdx_Phys_S.
> >
> > Fixes: 48da29e4 ("target/arm: Add ptw_idx to S1Translate")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1293
> > Signed-off-by: Richard Henderson
> > ---
> > target/arm/ptw.c | 15 ---
> > 1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> > index 58a7bbda50..df3573f150 100644
> > --- a/target/arm/ptw.c
> > +++ b/target/arm/ptw.c
> > @@ -1357,7 +1357,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> > S1Translate *ptw,
> > descaddr |= (address >> (stride * (4 - level))) & indexmask;
> > descaddr &= ~7ULL;
> > nstable = extract32(tableattrs, 4, 1);
> > -if (!nstable) {
> > +if (nstable) {
> > /*
> > * Stage2_S -> Stage2 or Phys_S -> Phys_NS
> > * Assert that the non-secure idx are even, and relative order.
> > @@ -2671,6 +2671,13 @@ static bool get_phys_addr_with_struct(CPUARMState
> > *env, S1Translate *ptw,
> > bool is_secure = ptw->in_secure;
> > ARMMMUIdx s1_mmu_idx;
> >
> > +/*
> > + * The page table entries may downgrade secure to non-secure, but
> > + * cannot upgrade an non-secure translation regime's attributes
> > + * to secure.
> > + */
> > +result->f.attrs.secure = is_secure;
> > +
> > switch (mmu_idx) {
> > case ARMMMUIdx_Phys_S:
> > case ARMMMUIdx_Phys_NS:
> > @@ -2712,12 +2719,6 @@ static bool get_phys_addr_with_struct(CPUARMState
> > *env, S1Translate *ptw,
> > break;
> > }
> >
> > -/*
> > - * The page table entries may downgrade secure to non-secure, but
> > - * cannot upgrade an non-secure translation regime's attributes
> > - * to secure.
> > - */
> > -result->f.attrs.secure = is_secure;
> > result->f.attrs.user = regime_is_user(env, mmu_idx);
>
> Do we also need to move this setting of attrs.user ?
> get_phys_addr_disabled() doesn't set that either.
I've applied this to target-arm.next for the moment anyway, since
it is definitely fixing an attrs.secure related bug. I can replace
that with a v2 or we can do a follow-on patch, depending whether
you get to this before or after I send out a pullreq.
thanks
-- PMM
Re: [PULL v2 00/82] pci,pc,virtio: features, tests, fixes, cleanups
On Thu, 3 Nov 2022 at 08:14, Michael S. Tsirkin wrote: > On Wed, Nov 02, 2022 at 03:47:43PM -0400, Stefan Hajnoczi wrote: > > On Wed, Nov 02, 2022 at 12:02:14PM -0400, Michael S. Tsirkin wrote: > > > Changes from v1: > > > > > > Applied and squashed fixes by Igor, Lei He, Hesham Almatary for > > > bugs that tripped up the pipeline. > > > Updated expected files for core-count test. > > > > Several "make check" CI failures have occurred. They look like they are > > related. Here is one (see the URLs at the bottom of this email for more > > details): > > > > 17/106 ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child > > process > > (/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch/subprocess > > [8609]) failed unexpectedly ERROR > > 17/106 qemu:qtest+qtest-arm / qtest-arm/qos-test ERROR > > 31.44s killed by signal 6 SIGABRT > > >>> G_TEST_DBUS_DAEMON=/builds/qemu-project/qemu/tests/dbus-vmstate-daemon.sh > > >>> MALLOC_PERTURB_=49 QTEST_QEMU_IMG=./qemu-img > > >>> QTEST_QEMU_BINARY=./qemu-system-arm > > >>> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon > > >>> /builds/qemu-project/qemu/build/tests/qtest/qos-test --tap -k > > ― ✀ > > ― > > stderr: > > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) > > qemu-system-arm: Failed to set msg fds. > > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) > > qemu-system-arm: -chardev > > socket,id=chr-reconnect,path=/tmp/vhost-test-6PT2U1/reconnect.sock,server=on: > > info: QEMU waiting for connection on: > > disconnected:unix:/tmp/vhost-test-6PT2U1/reconnect.sock,server=on > > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) > > qemu-system-arm: Failed to set msg fds. > > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) > > qemu-system-arm: -chardev > > socket,id=chr-connect-fail,path=/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on: > > info: QEMU waiting for connection on: > > disconnected:unix:/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on > > qemu-system-arm: -netdev > > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: Failed to read > > msg header. Read 0 instead of 12. Original request 1. > > qemu-system-arm: -netdev > > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: > > vhost_backend_init failed: Protocol error > > qemu-system-arm: -netdev > > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: failed to init > > vhost_net for queue 0 > > qemu-system-arm: -netdev > > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: info: QEMU > > waiting for connection on: > > disconnected:unix:/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on > > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) > > qemu-system-arm: Failed to set msg fds. > > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) > > qemu-system-arm: -chardev > > socket,id=chr-flags-mismatch,path=/tmp/vhost-test-94UYU1/flags-mismatch.sock,server=on: > > info: QEMU waiting for connection on: > > disconnected:unix:/tmp/vhost-test-94UYU1/flags-mismatch.sock,server=on > > qemu-system-arm: Failed to write msg. Wrote -1 instead of 52. > > qemu-system-arm: vhost_set_mem_table failed: Invalid argument (22) > > qemu-system-arm: Failed to set msg fds. > > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) > > UndefinedBehaviorSanitizer:DEADLYSIGNAL > > ==8618==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address > > 0x (pc 0x55e34deccab0 bp 0x sp 0x7ffc94894710 T8618) > > ==8618==The signal is caused by a READ memory access. > > ==8618==Hint: address points to the zero page. > > #0 0x55e34deccab0 in ldl_he_p > > /builds/qemu-project/qemu/include/qemu/bswap.h:301:5 > > #1 0x55e34deccab0 in ldn_he_p > > /builds/qemu-project/qemu/include/qemu/bswap.h:440:1 > > #2 0x55e34deccab0 in flatview_write_continue > > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2824:19 > > #3 0x55e34dec9f21 in flatview_write > > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2867:12 > > #4 0x55e34dec9f21 in address_space_write > > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2963:18 > > #5 0x55e34decace7 in address_space_unmap > > /builds/qemu-project/qemu/build/../softmmu/physmem.c:3306:9 > > #6 0x55e34de6d4ec in vhost_memory_unmap > > /builds/qemu-project/qemu/build/../hw/virtio/vhost.c:342:9 > > #7 0x55e34de6d4ec in vhost_virtqueue_stop > > /builds/qemu-project/qemu/build/../hw/virtio/vhost.c:1242:5 > > #8 0x55e34de
Re: [PULL v2 00/82] pci,pc,virtio: features, tests, fixes, cleanups
On 3/11/22 13:13, Michael S. Tsirkin wrote: On Wed, Nov 02, 2022 at 03:47:43PM -0400, Stefan Hajnoczi wrote: On Wed, Nov 02, 2022 at 12:02:14PM -0400, Michael S. Tsirkin wrote: Changes from v1: Applied and squashed fixes by Igor, Lei He, Hesham Almatary for bugs that tripped up the pipeline. Updated expected files for core-count test. Several "make check" CI failures have occurred. They look like they are related. Here is one (see the URLs at the bottom of this email for more details): 17/106 ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child process (/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch/subprocess [8609]) failed unexpectedly ERROR 17/106 qemu:qtest+qtest-arm / qtest-arm/qos-test ERROR 31.44s killed by signal 6 SIGABRT G_TEST_DBUS_DAEMON=/builds/qemu-project/qemu/tests/dbus-vmstate-daemon.sh MALLOC_PERTURB_=49 QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-arm QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon /builds/qemu-project/qemu/build/tests/qtest/qos-test --tap -k ― ✀ ― stderr: qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-reconnect,path=/tmp/vhost-test-6PT2U1/reconnect.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-6PT2U1/reconnect.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-connect-fail,path=/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: Failed to read msg header. Read 0 instead of 12. Original request 1. qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: vhost_backend_init failed: Protocol error qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: failed to init vhost_net for queue 0 qemu-system-arm: -netdev vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument (22) qemu-system-arm: -chardev socket,id=chr-flags-mismatch,path=/tmp/vhost-test-94UYU1/flags-mismatch.sock,server=on: info: QEMU waiting for connection on: disconnected:unix:/tmp/vhost-test-94UYU1/flags-mismatch.sock,server=on qemu-system-arm: Failed to write msg. Wrote -1 instead of 52. qemu-system-arm: vhost_set_mem_table failed: Invalid argument (22) qemu-system-arm: Failed to set msg fds. qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument (22) UndefinedBehaviorSanitizer:DEADLYSIGNAL ==8618==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address 0x (pc 0x55e34deccab0 bp 0x sp 0x7ffc94894710 T8618) ==8618==The signal is caused by a READ memory access. ==8618==Hint: address points to the zero page. #0 0x55e34deccab0 in ldl_he_p /builds/qemu-project/qemu/include/qemu/bswap.h:301:5 #1 0x55e34deccab0 in ldn_he_p /builds/qemu-project/qemu/include/qemu/bswap.h:440:1 #2 0x55e34deccab0 in flatview_write_continue /builds/qemu-project/qemu/build/../softmmu/physmem.c:2824:19 #3 0x55e34dec9f21 in flatview_write /builds/qemu-project/qemu/build/../softmmu/physmem.c:2867:12 #4 0x55e34dec9f21 in address_space_write /builds/qemu-project/qemu/build/../softmmu/physmem.c:2963:18 #5 0x55e34decace7 in address_space_unmap /builds/qemu-project/qemu/build/../softmmu/physmem.c:3306:9 #6 0x55e34de6d4ec in vhost_memory_unmap /builds/qemu-project/qemu/build/../hw/virtio/vhost.c:342:9 #7 0x55e34de6d4ec in vhost_virtqueue_stop /builds/qemu-project/qemu/build/../hw/virtio/vhost.c:1242:5 #8 0x55e34de72904 in vhost_dev_stop /builds/qemu-project/qemu/build/../hw/virtio/vhost.c:1882:9 #9 0x55e34d890514 in vhost_net_stop_one /builds/qemu-project/qemu/build/../hw/net/vhost_net.c:331:5 #10 0x55e34d88fef6 in vhost_net_start /builds/qemu-project/qemu/build/../hw/net/vhost_net.c:404:13 #11 0x55e34de0bec6 in virtio_net_vhost_status /builds/qemu-project/qemu/bui
Re: [PATCH 12/26] target/s390x: Move masking of psw.addr to cpu_get_tb_cpu_state
On Wed, Oct 05, 2022 at 08:44:07PM -0700, Richard Henderson wrote: > Masking after the fact in s390x_tr_init_disas_context > provides incorrect information to tb_lookup. > > Signed-off-by: Richard Henderson > --- > target/s390x/cpu.h | 13 +++-- > target/s390x/tcg/translate.c | 6 -- > 2 files changed, 7 insertions(+), 12 deletions(-) How can we end up in a situation where this matters? E.g. if we are in 64-bit mode and execute 0xa12345678: sam31 we will get a specification exception, and cpu_get_tb_cpu_state() will not run. And for valid 31-bit addresses masking should be a no-op.
[PATCH 6/9] block/vmdk: add missing coroutine_fn annotations
vmdk_co_create_opts() is a coroutine_fn, and calls vmdk_co_do_create()
which in turn can call two callbacks: vmdk_co_create_opts_cb and
vmdk_co_create_cb.
Mark all these functions as coroutine_fn, since vmdk_co_create_opts()
is the only caller.
Signed-off-by: Emanuele Giuseppe Esposito
---
block/vmdk.c | 36 +++-
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 26376352b9..0c32bf2e83 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2285,10 +2285,11 @@ exit:
return ret;
}
-static int vmdk_create_extent(const char *filename, int64_t filesize,
- bool flat, bool compress, bool zeroed_grain,
- BlockBackend **pbb,
- QemuOpts *opts, Error **errp)
+static int coroutine_fn vmdk_create_extent(const char *filename,
+ int64_t filesize, bool flat,
+ bool compress, bool zeroed_grain,
+ BlockBackend **pbb,
+ QemuOpts *opts, Error **errp)
{
int ret;
BlockBackend *blk = NULL;
@@ -2366,14 +2367,14 @@ static int filename_decompose(const char *filename,
char *path, char *prefix,
* non-split format.
* idx >= 1: get the n-th extent if in a split subformat
*/
-typedef BlockBackend *(*vmdk_create_extent_fn)(int64_t size,
- int idx,
- bool flat,
- bool split,
- bool compress,
- bool zeroed_grain,
- void *opaque,
- Error **errp);
+typedef BlockBackend * coroutine_fn (*vmdk_create_extent_fn)(int64_t size,
+ int idx,
+ bool flat,
+ bool split,
+ bool compress,
+ bool zeroed_grain,
+ void *opaque,
+ Error **errp);
static void vmdk_desc_add_extent(GString *desc,
const char *extent_line_fmt,
@@ -2616,7 +2617,7 @@ typedef struct {
QemuOpts *opts;
} VMDKCreateOptsData;
-static BlockBackend *vmdk_co_create_opts_cb(int64_t size, int idx,
+static BlockBackend * coroutine_fn vmdk_co_create_opts_cb(int64_t size, int
idx,
bool flat, bool split, bool
compress,
bool zeroed_grain, void *opaque,
Error **errp)
@@ -2768,10 +2769,11 @@ exit:
return ret;
}
-static BlockBackend *vmdk_co_create_cb(int64_t size, int idx,
- bool flat, bool split, bool compress,
- bool zeroed_grain, void *opaque,
- Error **errp)
+static BlockBackend * coroutine_fn vmdk_co_create_cb(int64_t size, int idx,
+ bool flat, bool split,
+ bool compress,
+ bool zeroed_grain,
+ void *opaque, Error
**errp)
{
int ret;
BlockDriverState *bs;
--
2.31.1
[PATCH 9/9] block/dirty-bitmap: remove unnecessary qemu_in_coroutine() case
bdrv_can_store_new_dirty_bitmap and bdrv_remove_persistent_dirty_bitmap
check if they are running in a coroutine, directly calling the
coroutine callback if it's the case.
Except that no coroutine calls such functions, therefore that check
can be removed.
Signed-off-by: Emanuele Giuseppe Esposito
---
block/dirty-bitmap.c | 66 +++-
1 file changed, 29 insertions(+), 37 deletions(-)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index bf3dc0512a..8092d08261 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -418,24 +418,20 @@ bdrv_co_remove_persistent_dirty_bitmap_entry(void *opaque)
int bdrv_remove_persistent_dirty_bitmap(BlockDriverState *bs, const char *name,
Error **errp)
{
-if (qemu_in_coroutine()) {
-return bdrv_co_remove_persistent_dirty_bitmap(bs, name, errp);
-} else {
-Coroutine *co;
-BdrvRemovePersistentDirtyBitmapCo s = {
-.bs = bs,
-.name = name,
-.errp = errp,
-.ret = -EINPROGRESS,
-};
-
-co =
qemu_coroutine_create(bdrv_co_remove_persistent_dirty_bitmap_entry,
- &s);
-bdrv_coroutine_enter(bs, co);
-BDRV_POLL_WHILE(bs, s.ret == -EINPROGRESS);
-
-return s.ret;
-}
+Coroutine *co;
+BdrvRemovePersistentDirtyBitmapCo s = {
+.bs = bs,
+.name = name,
+.errp = errp,
+.ret = -EINPROGRESS,
+};
+assert(!qemu_in_coroutine());
+co = qemu_coroutine_create(bdrv_co_remove_persistent_dirty_bitmap_entry,
+&s);
+bdrv_coroutine_enter(bs, co);
+BDRV_POLL_WHILE(bs, s.ret == -EINPROGRESS);
+
+return s.ret;
}
bool
@@ -494,25 +490,21 @@ bool bdrv_can_store_new_dirty_bitmap(BlockDriverState
*bs, const char *name,
uint32_t granularity, Error **errp)
{
IO_CODE();
-if (qemu_in_coroutine()) {
-return bdrv_co_can_store_new_dirty_bitmap(bs, name, granularity, errp);
-} else {
-Coroutine *co;
-BdrvCanStoreNewDirtyBitmapCo s = {
-.bs = bs,
-.name = name,
-.granularity = granularity,
-.errp = errp,
-.in_progress = true,
-};
-
-co = qemu_coroutine_create(bdrv_co_can_store_new_dirty_bitmap_entry,
- &s);
-bdrv_coroutine_enter(bs, co);
-BDRV_POLL_WHILE(bs, s.in_progress);
-
-return s.ret;
-}
+Coroutine *co;
+BdrvCanStoreNewDirtyBitmapCo s = {
+.bs = bs,
+.name = name,
+.granularity = granularity,
+.errp = errp,
+.in_progress = true,
+};
+assert(!qemu_in_coroutine());
+co = qemu_coroutine_create(bdrv_co_can_store_new_dirty_bitmap_entry,
+&s);
+bdrv_coroutine_enter(bs, co);
+BDRV_POLL_WHILE(bs, s.in_progress);
+
+return s.ret;
}
void bdrv_disable_dirty_bitmap(BdrvDirtyBitmap *bitmap)
--
2.31.1
[PATCH 2/9] block-copy: add missing coroutine_fn annotations
block_copy_reset_unallocated and block_copy_is_cluster_allocated are
only called by backup_run, a corotuine_fn itself.
Same applies to block_copy_block_status, called by
block_copy_dirty_clusters.
Therefore mark them as coroutine too.
Signed-off-by: Emanuele Giuseppe Esposito
---
block/block-copy.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index bb947afdda..f33ab1d0b6 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -577,8 +577,9 @@ static coroutine_fn int block_copy_task_entry(AioTask *task)
return ret;
}
-static int block_copy_block_status(BlockCopyState *s, int64_t offset,
- int64_t bytes, int64_t *pnum)
+static coroutine_fn int block_copy_block_status(BlockCopyState *s,
+int64_t offset,
+int64_t bytes, int64_t *pnum)
{
int64_t num;
BlockDriverState *base;
@@ -613,8 +614,9 @@ static int block_copy_block_status(BlockCopyState *s,
int64_t offset,
* Check if the cluster starting at offset is allocated or not.
* return via pnum the number of contiguous clusters sharing this allocation.
*/
-static int block_copy_is_cluster_allocated(BlockCopyState *s, int64_t offset,
- int64_t *pnum)
+static int coroutine_fn block_copy_is_cluster_allocated(BlockCopyState *s,
+int64_t offset,
+int64_t *pnum)
{
BlockDriverState *bs = s->source->bs;
int64_t count, total_count = 0;
@@ -669,8 +671,9 @@ void block_copy_reset(BlockCopyState *s, int64_t offset,
int64_t bytes)
* @return 0 when the cluster at @offset was unallocated,
* 1 otherwise, and -ret on error.
*/
-int64_t block_copy_reset_unallocated(BlockCopyState *s,
- int64_t offset, int64_t *count)
+int64_t coroutine_fn block_copy_reset_unallocated(BlockCopyState *s,
+ int64_t offset,
+ int64_t *count)
{
int ret;
int64_t clusters, bytes;
--
2.31.1
[PATCH 0/9] Still more coroutine and various fixes in block layer
This is a dump of all minor coroutine-related fixes found while looking around and testing various things in the QEMU block layer. Patches aim to: - add missing coroutine_fn annotation to the functions - simplify to avoid the typical "if in coroutine: fn() // else create_coroutine(fn)" already present in generated_co_wraper functions. - make sure that if a BlockDriver callback is defined as coroutine_fn, then it is always running in a coroutine. Emanuele Emanuele Giuseppe Esposito (9): block: call bdrv_co_drain_begin in a coroutine block-copy: add missing coroutine_fn annotations nbd/server.c: add missing coroutine_fn annotations block-backend: replace bdrv_*_above with blk_*_above block: distinguish between bdrv_create running in coroutine and not block/vmdk: add missing coroutine_fn annotations block: bdrv_create_file is a coroutine_fn block: bdrv_create is never called in non-coroutine context block/dirty-bitmap: remove unnecessary qemu_in_coroutine() case block.c| 107 + block/block-backend.c | 21 ++ block/block-copy.c | 15 ++-- block/commit.c | 4 +- block/dirty-bitmap.c | 66 -- block/vmdk.c | 36 +- include/block/block-global-state.h | 3 +- include/sysemu/block-backend-io.h | 9 +++ nbd/server.c | 43 ++-- qemu-img.c | 4 +- 10 files changed, 178 insertions(+), 130 deletions(-) -- 2.31.1
[PATCH 3/9] nbd/server.c: add missing coroutine_fn annotations
There are probably more missing, but right now it is necessary that
we extend coroutine_fn to block{allock/status}_to_extents, because
they use bdrv_* functions calling the generated_co_wrapper API, which
checks for the qemu_in_coroutine() case.
Signed-off-by: Emanuele Giuseppe Esposito
---
nbd/server.c | 21 -
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index ada16089f3..e2eec0cf46 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2141,8 +2141,9 @@ static int nbd_extent_array_add(NBDExtentArray *ea,
return 0;
}
-static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, NBDExtentArray *ea)
+static int coroutine_fn blockstatus_to_extents(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ NBDExtentArray *ea)
{
while (bytes) {
uint32_t flags;
@@ -2168,8 +2169,9 @@ static int blockstatus_to_extents(BlockDriverState *bs,
uint64_t offset,
return 0;
}
-static int blockalloc_to_extents(BlockDriverState *bs, uint64_t offset,
- uint64_t bytes, NBDExtentArray *ea)
+static int coroutine_fn blockalloc_to_extents(BlockDriverState *bs,
+ uint64_t offset, uint64_t bytes,
+ NBDExtentArray *ea)
{
while (bytes) {
int64_t num;
@@ -2220,11 +,12 @@ static int nbd_co_send_extents(NBDClient *client,
uint64_t handle,
}
/* Get block status from the exported device and send it to the client */
-static int nbd_co_send_block_status(NBDClient *client, uint64_t handle,
-BlockDriverState *bs, uint64_t offset,
-uint32_t length, bool dont_fragment,
-bool last, uint32_t context_id,
-Error **errp)
+static int
+coroutine_fn nbd_co_send_block_status(NBDClient *client, uint64_t handle,
+ BlockDriverState *bs, uint64_t offset,
+ uint32_t length, bool dont_fragment,
+ bool last, uint32_t context_id,
+ Error **errp)
{
int ret;
unsigned int nb_extents = dont_fragment ? 1 : NBD_MAX_BLOCK_STATUS_EXTENTS;
--
2.31.1
[PATCH 5/9] block: distinguish between bdrv_create running in coroutine and not
Call two different functions depending on whether bdrv_create
is in coroutine or not, following the same pattern as
generated_co_wrapper functions.
This allows to also call the coroutine function directly,
without using CreateCo or relying in bdrv_create().
Signed-off-by: Emanuele Giuseppe Esposito
---
block.c | 74 -
1 file changed, 36 insertions(+), 38 deletions(-)
diff --git a/block.c b/block.c
index 7211f62001..eeb7a02aa2 100644
--- a/block.c
+++ b/block.c
@@ -522,66 +522,64 @@ typedef struct CreateCo {
Error *err;
} CreateCo;
-static void coroutine_fn bdrv_create_co_entry(void *opaque)
+static int coroutine_fn bdrv_co_create(BlockDriver *drv, const char *filename,
+ QemuOpts *opts, Error **errp)
{
-Error *local_err = NULL;
int ret;
+GLOBAL_STATE_CODE();
+assert(*errp == NULL);
+
+if (!drv->bdrv_co_create_opts) {
+error_setg(errp, "Driver '%s' does not support image creation",
+ drv->format_name);
+return -ENOTSUP;
+}
+
+ret = drv->bdrv_co_create_opts(drv, filename, opts, errp);
+if (ret < 0 && !*errp) {
+error_setg_errno(errp, -ret, "Could not create image");
+}
+
+return ret;
+}
+
+static void coroutine_fn bdrv_create_co_entry(void *opaque)
+{
CreateCo *cco = opaque;
-assert(cco->drv);
GLOBAL_STATE_CODE();
+assert(cco->drv);
-ret = cco->drv->bdrv_co_create_opts(cco->drv,
-cco->filename, cco->opts, &local_err);
-error_propagate(&cco->err, local_err);
-cco->ret = ret;
+cco->ret = bdrv_co_create(cco->drv, cco->filename, cco->opts, &cco->err);
}
int bdrv_create(BlockDriver *drv, const char* filename,
QemuOpts *opts, Error **errp)
{
-int ret;
-
GLOBAL_STATE_CODE();
-Coroutine *co;
-CreateCo cco = {
-.drv = drv,
-.filename = g_strdup(filename),
-.opts = opts,
-.ret = NOT_DONE,
-.err = NULL,
-};
-
-if (!drv->bdrv_co_create_opts) {
-error_setg(errp, "Driver '%s' does not support image creation",
drv->format_name);
-ret = -ENOTSUP;
-goto out;
-}
-
if (qemu_in_coroutine()) {
/* Fast-path if already in coroutine context */
-bdrv_create_co_entry(&cco);
+return bdrv_co_create(drv, filename, opts, errp);
} else {
+Coroutine *co;
+CreateCo cco = {
+.drv = drv,
+.filename = g_strdup(filename),
+.opts = opts,
+.ret = NOT_DONE,
+.err = NULL,
+};
+
co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
qemu_coroutine_enter(co);
while (cco.ret == NOT_DONE) {
aio_poll(qemu_get_aio_context(), true);
}
+error_propagate(errp, cco.err);
+g_free(cco.filename);
+return cco.ret;
}
-
-ret = cco.ret;
-if (ret < 0) {
-if (cco.err) {
-error_propagate(errp, cco.err);
-} else {
-error_setg_errno(errp, -ret, "Could not create image");
-}
-}
-
-out:
-g_free(cco.filename);
-return ret;
}
/**
--
2.31.1
[PATCH 4/9] block-backend: replace bdrv_*_above with blk_*_above
Avoid mixing bdrv_* functions with blk_*, so create blk_* counterparts
for:
- bdrv_block_status_above
- bdrv_is_allocated_above
Signed-off-by: Emanuele Giuseppe Esposito
---
block/block-backend.c | 21 +
block/commit.c| 4 ++--
include/sysemu/block-backend-io.h | 9 +
nbd/server.c | 28 ++--
qemu-img.c| 4 ++--
5 files changed, 48 insertions(+), 18 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index ec17dc49a9..eb8787a3d9 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1423,6 +1423,27 @@ int coroutine_fn blk_co_pwritev(BlockBackend *blk,
int64_t offset,
return blk_co_pwritev_part(blk, offset, bytes, qiov, 0, flags);
}
+int coroutine_fn blk_block_status_above(BlockBackend *blk,
+BlockDriverState *base,
+int64_t offset, int64_t bytes,
+int64_t *pnum, int64_t *map,
+BlockDriverState **file)
+{
+IO_CODE();
+return bdrv_block_status_above(blk_bs(blk), base, offset, bytes, pnum, map,
+ file);
+}
+
+int coroutine_fn blk_is_allocated_above(BlockBackend *blk,
+BlockDriverState *base,
+bool include_base, int64_t offset,
+int64_t bytes, int64_t *pnum)
+{
+IO_CODE();
+return bdrv_is_allocated_above(blk_bs(blk), base, include_base, offset,
+ bytes, pnum);
+}
+
typedef struct BlkRwCo {
BlockBackend *blk;
int64_t offset;
diff --git a/block/commit.c b/block/commit.c
index 0029b31944..9d4d908344 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -155,8 +155,8 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
break;
}
/* Copy if allocated above the base */
-ret = bdrv_is_allocated_above(blk_bs(s->top), s->base_overlay, true,
- offset, COMMIT_BUFFER_SIZE, &n);
+ret = blk_is_allocated_above(s->top, s->base_overlay, true,
+ offset, COMMIT_BUFFER_SIZE, &n);
copy = (ret > 0);
trace_commit_one_iteration(s, offset, n, ret);
if (copy) {
diff --git a/include/sysemu/block-backend-io.h
b/include/sysemu/block-backend-io.h
index 50f5aa2e07..a47cb825e5 100644
--- a/include/sysemu/block-backend-io.h
+++ b/include/sysemu/block-backend-io.h
@@ -92,6 +92,15 @@ int coroutine_fn blk_co_copy_range(BlockBackend *blk_in,
int64_t off_in,
int64_t bytes, BdrvRequestFlags read_flags,
BdrvRequestFlags write_flags);
+int coroutine_fn blk_block_status_above(BlockBackend *blk,
+BlockDriverState *base,
+int64_t offset, int64_t bytes,
+int64_t *pnum, int64_t *map,
+BlockDriverState **file);
+int coroutine_fn blk_is_allocated_above(BlockBackend *blk,
+BlockDriverState *base,
+bool include_base, int64_t offset,
+int64_t bytes, int64_t *pnum);
/*
* "I/O or GS" API functions. These functions can run without
diff --git a/nbd/server.c b/nbd/server.c
index e2eec0cf46..6389b46396 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1991,7 +1991,7 @@ static int coroutine_fn
nbd_co_send_structured_error(NBDClient *client,
}
/* Do a sparse read and send the structured reply to the client.
- * Returns -errno if sending fails. bdrv_block_status_above() failure is
+ * Returns -errno if sending fails. blk_block_status_above() failure is
* reported to the client, at which point this function succeeds.
*/
static int coroutine_fn nbd_co_send_sparse_read(NBDClient *client,
@@ -2007,10 +2007,10 @@ static int coroutine_fn
nbd_co_send_sparse_read(NBDClient *client,
while (progress < size) {
int64_t pnum;
-int status = bdrv_block_status_above(blk_bs(exp->common.blk), NULL,
- offset + progress,
- size - progress, &pnum, NULL,
- NULL);
+int status = blk_block_status_above(exp->common.blk, NULL,
+offset + progress,
+size - progress, &pnum, NULL,
+NULL);
bool final;
if (status < 0) {
@@ -2141,14 +2141,14 @@ static int nbd_extent_array_add(NBDExtentArray *ea,
return 0;
}
-static int coroutine_f
[PATCH 8/9] block: bdrv_create is never called in non-coroutine context
Delete the if case and make sure it won't be called again
in coroutines.
Signed-off-by: Emanuele Giuseppe Esposito
---
block.c | 37 -
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/block.c b/block.c
index e5e70acf15..1ee76a8694 100644
--- a/block.c
+++ b/block.c
@@ -557,30 +557,25 @@ static void coroutine_fn bdrv_create_co_entry(void
*opaque)
int bdrv_create(BlockDriver *drv, const char* filename,
QemuOpts *opts, Error **errp)
{
+Coroutine *co;
+CreateCo cco = {
+.drv = drv,
+.filename = g_strdup(filename),
+.opts = opts,
+.ret = NOT_DONE,
+.err = NULL,
+};
GLOBAL_STATE_CODE();
+assert(!qemu_in_coroutine());
-if (qemu_in_coroutine()) {
-/* Fast-path if already in coroutine context */
-return bdrv_co_create(drv, filename, opts, errp);
-} else {
-Coroutine *co;
-CreateCo cco = {
-.drv = drv,
-.filename = g_strdup(filename),
-.opts = opts,
-.ret = NOT_DONE,
-.err = NULL,
-};
-
-co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
-qemu_coroutine_enter(co);
-while (cco.ret == NOT_DONE) {
-aio_poll(qemu_get_aio_context(), true);
-}
-error_propagate(errp, cco.err);
-g_free(cco.filename);
-return cco.ret;
+co = qemu_coroutine_create(bdrv_create_co_entry, &cco);
+qemu_coroutine_enter(co);
+while (cco.ret == NOT_DONE) {
+aio_poll(qemu_get_aio_context(), true);
}
+error_propagate(errp, cco.err);
+g_free(cco.filename);
+return cco.ret;
}
/**
--
2.31.1
[PATCH 7/9] block: bdrv_create_file is a coroutine_fn
It is always called in coroutine_fn callbacks, therefore
it can directly call bdrv_co_create().
Signed-off-by: Emanuele Giuseppe Esposito
---
block.c| 6 --
include/block/block-global-state.h | 3 ++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/block.c b/block.c
index eeb7a02aa2..e5e70acf15 100644
--- a/block.c
+++ b/block.c
@@ -527,6 +527,7 @@ static int coroutine_fn bdrv_co_create(BlockDriver *drv,
const char *filename,
{
int ret;
GLOBAL_STATE_CODE();
+assert(qemu_in_coroutine());
assert(*errp == NULL);
if (!drv->bdrv_co_create_opts) {
@@ -717,7 +718,8 @@ out:
return ret;
}
-int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp)
+int coroutine_fn bdrv_create_file(const char *filename, QemuOpts *opts,
+ Error **errp)
{
QemuOpts *protocol_opts;
BlockDriver *drv;
@@ -758,7 +760,7 @@ int bdrv_create_file(const char *filename, QemuOpts *opts,
Error **errp)
goto out;
}
-ret = bdrv_create(drv, filename, protocol_opts, errp);
+ret = bdrv_co_create(drv, filename, protocol_opts, errp);
out:
qemu_opts_del(protocol_opts);
qobject_unref(qdict);
diff --git a/include/block/block-global-state.h
b/include/block/block-global-state.h
index 73795a0095..bd461f06a1 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -57,7 +57,8 @@ BlockDriver *bdrv_find_protocol(const char *filename,
BlockDriver *bdrv_find_format(const char *format_name);
int bdrv_create(BlockDriver *drv, const char* filename,
QemuOpts *opts, Error **errp);
-int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp);
+int coroutine_fn bdrv_create_file(const char *filename, QemuOpts *opts,
+ Error **errp);
BlockDriverState *bdrv_new(void);
int bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
--
2.31.1
[PATCH 1/9] block: call bdrv_co_drain_begin in a coroutine
It seems that bdrv_open_driver() forgot to create a coroutine
where to call bs->drv->bdrv_co_drain_begin(), a callback
marked as coroutine_fn.
Signed-off-by: Emanuele Giuseppe Esposito
---
block.c | 32 +++-
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/block.c b/block.c
index 5311b21f8e..7211f62001 100644
--- a/block.c
+++ b/block.c
@@ -1637,12 +1637,34 @@ out:
g_free(gen_node_name);
}
+typedef struct DrainCo {
+BlockDriverState *bs;
+int ret;
+} DrainCo;
+
+static void coroutine_fn bdrv_co_drain_begin(void *opaque)
+{
+int i;
+DrainCo *co = opaque;
+BlockDriverState *bs = co->bs;
+
+for (i = 0; i < bs->quiesce_counter; i++) {
+bs->drv->bdrv_co_drain_begin(bs);
+}
+co->ret = 0;
+}
+
static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv,
const char *node_name, QDict *options,
int open_flags, Error **errp)
{
Error *local_err = NULL;
-int i, ret;
+int ret;
+Coroutine *co;
+DrainCo dco = {
+.bs = bs,
+.ret = NOT_DONE,
+};
GLOBAL_STATE_CODE();
bdrv_assign_node_name(bs, node_name, &local_err);
@@ -1690,10 +1712,10 @@ static int bdrv_open_driver(BlockDriverState *bs,
BlockDriver *drv,
assert(bdrv_min_mem_align(bs) != 0);
assert(is_power_of_2(bs->bl.request_alignment));
-for (i = 0; i < bs->quiesce_counter; i++) {
-if (drv->bdrv_co_drain_begin) {
-drv->bdrv_co_drain_begin(bs);
-}
+if (drv->bdrv_co_drain_begin) {
+co = qemu_coroutine_create(bdrv_co_drain_begin, &dco);
+qemu_coroutine_enter(co);
+AIO_WAIT_WHILE(qemu_get_aio_context(), dco.ret == NOT_DONE);
}
return 0;
--
2.31.1
Re: [PATCH 13/26] target/s390x: Add disp argument to update_psw_addr
On Wed, Oct 05, 2022 at 08:44:08PM -0700, Richard Henderson wrote: > Rename to update_psw_addr_disp at the same time. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 22 +++--- > 1 file changed, 11 insertions(+), 11 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH 14/26] target/s390x: Don't set gbea for user-only
On Wed, Oct 05, 2022 at 08:44:09PM -0700, Richard Henderson wrote: > The rest of the per_* functions have this ifdef; > this one seemed to be missing. > > Signed-off-by: Richard Henderson Reviewed-by: Ilya Leoshkevich
Re: [PATCH 15/26] target/s390x: Introduce per_enabled
On Wed, Oct 05, 2022 at 08:44:10PM -0700, Richard Henderson wrote: > Hoist the test of FLAG_MASK_PER to a helper. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 23 --- > 1 file changed, 16 insertions(+), 7 deletions(-) Reviewed-by: Ilya Leoshkevich
[PATCH RFC] target/loongarch: Fix emulation of float-point disable exception
We need to emulate it to generate a floating point disable exception
when CSR.EUEN.FPE is zero.
Signed-off-by: Rui Wang
---
target/loongarch/cpu.c| 2 ++
.../loongarch/insn_trans/trans_farith.c.inc | 36 +++
target/loongarch/insn_trans/trans_fcmp.c.inc | 5 +++
.../loongarch/insn_trans/trans_fmemory.c.inc | 16 +
target/loongarch/insn_trans/trans_fmov.c.inc | 20 +++
target/loongarch/translate.c | 4 +++
target/loongarch/translate.h | 1 +
7 files changed, 84 insertions(+)
diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c
index 49393d95d8..abe4a819b2 100644
--- a/target/loongarch/cpu.c
+++ b/target/loongarch/cpu.c
@@ -48,6 +48,7 @@ static const char * const excp_names[] = {
[EXCCODE_BRK] = "Break",
[EXCCODE_INE] = "Instruction Non-Existent",
[EXCCODE_IPE] = "Instruction privilege error",
+[EXCCODE_FPD] = "Floating Point Disabled",
[EXCCODE_FPE] = "Floating Point Exception",
[EXCCODE_DBP] = "Debug breakpoint",
[EXCCODE_BCE] = "Bound Check Exception",
@@ -184,6 +185,7 @@ static void loongarch_cpu_do_interrupt(CPUState *cs)
case EXCCODE_BRK:
case EXCCODE_INE:
case EXCCODE_IPE:
+case EXCCODE_FPD:
case EXCCODE_FPE:
case EXCCODE_BCE:
env->CSR_BADV = env->pc;
diff --git a/target/loongarch/insn_trans/trans_farith.c.inc
b/target/loongarch/insn_trans/trans_farith.c.inc
index 7bb3f41aee..3574b9b746 100644
--- a/target/loongarch/insn_trans/trans_farith.c.inc
+++ b/target/loongarch/insn_trans/trans_farith.c.inc
@@ -3,9 +3,28 @@
* Copyright (c) 2021 Loongson Technology Corporation Limited
*/
+static void check_fpe(DisasContext *ctx)
+{
+#ifndef CONFIG_USER_ONLY
+TCGLabel *skip = gen_new_label();
+TCGv tmp = tcg_temp_new();
+
+tcg_gen_andi_tl(tmp, cpu_euen, R_CSR_EUEN_FPE_MASK);
+tcg_gen_brcond_tl(TCG_COND_NE, tmp, ctx->zero, skip);
+tcg_temp_free(tmp);
+
+generate_exception(ctx, EXCCODE_FPD);
+ctx->base.is_jmp = DISAS_EXIT_UPDATE;
+
+gen_set_label(skip);
+#endif
+}
+
static bool gen_fff(DisasContext *ctx, arg_fff *a,
void (*func)(TCGv, TCGv_env, TCGv, TCGv))
{
+check_fpe(ctx);
+
func(cpu_fpr[a->fd], cpu_env, cpu_fpr[a->fj], cpu_fpr[a->fk]);
return true;
}
@@ -13,6 +32,8 @@ static bool gen_fff(DisasContext *ctx, arg_fff *a,
static bool gen_ff(DisasContext *ctx, arg_ff *a,
void (*func)(TCGv, TCGv_env, TCGv))
{
+check_fpe(ctx);
+
func(cpu_fpr[a->fd], cpu_env, cpu_fpr[a->fj]);
return true;
}
@@ -22,6 +43,9 @@ static bool gen_muladd(DisasContext *ctx, arg_ *a,
int flag)
{
TCGv_i32 tflag = tcg_constant_i32(flag);
+
+check_fpe(ctx);
+
func(cpu_fpr[a->fd], cpu_env, cpu_fpr[a->fj],
cpu_fpr[a->fk], cpu_fpr[a->fa], tflag);
return true;
@@ -29,18 +53,24 @@ static bool gen_muladd(DisasContext *ctx, arg_ *a,
static bool trans_fcopysign_s(DisasContext *ctx, arg_fcopysign_s *a)
{
+check_fpe(ctx);
+
tcg_gen_deposit_i64(cpu_fpr[a->fd], cpu_fpr[a->fk], cpu_fpr[a->fj], 0, 31);
return true;
}
static bool trans_fcopysign_d(DisasContext *ctx, arg_fcopysign_d *a)
{
+check_fpe(ctx);
+
tcg_gen_deposit_i64(cpu_fpr[a->fd], cpu_fpr[a->fk], cpu_fpr[a->fj], 0, 63);
return true;
}
static bool trans_fabs_s(DisasContext *ctx, arg_fabs_s *a)
{
+check_fpe(ctx);
+
tcg_gen_andi_i64(cpu_fpr[a->fd], cpu_fpr[a->fj], MAKE_64BIT_MASK(0, 31));
gen_nanbox_s(cpu_fpr[a->fd], cpu_fpr[a->fd]);
return true;
@@ -48,12 +78,16 @@ static bool trans_fabs_s(DisasContext *ctx, arg_fabs_s *a)
static bool trans_fabs_d(DisasContext *ctx, arg_fabs_d *a)
{
+check_fpe(ctx);
+
tcg_gen_andi_i64(cpu_fpr[a->fd], cpu_fpr[a->fj], MAKE_64BIT_MASK(0, 63));
return true;
}
static bool trans_fneg_s(DisasContext *ctx, arg_fneg_s *a)
{
+check_fpe(ctx);
+
tcg_gen_xori_i64(cpu_fpr[a->fd], cpu_fpr[a->fj], 0x8000);
gen_nanbox_s(cpu_fpr[a->fd], cpu_fpr[a->fd]);
return true;
@@ -61,6 +95,8 @@ static bool trans_fneg_s(DisasContext *ctx, arg_fneg_s *a)
static bool trans_fneg_d(DisasContext *ctx, arg_fneg_d *a)
{
+check_fpe(ctx);
+
tcg_gen_xori_i64(cpu_fpr[a->fd], cpu_fpr[a->fj], 0x8000LL);
return true;
}
diff --git a/target/loongarch/insn_trans/trans_fcmp.c.inc
b/target/loongarch/insn_trans/trans_fcmp.c.inc
index 93a6a2230f..32b903231b 100644
--- a/target/loongarch/insn_trans/trans_fcmp.c.inc
+++ b/target/loongarch/insn_trans/trans_fcmp.c.inc
@@ -29,6 +29,8 @@ static bool trans_fcmp_cond_s(DisasContext *ctx,
arg_fcmp_cond_s *a)
uint32_t flags;
void (*fn)(TCGv, TCGv_env, TCGv, TCGv, TCGv_i32);
+check_fpe(ctx);
+
fn = (a->fcond & 1 ? gen_helper_fcmp_s_s : gen_helper_fcmp_c_s);
flags = get_fcmp_flags(a->fcond >> 1);
@@ -44,6 +46,9 @@ static bool trans_fcmp_cond_d(DisasContext *ctx,
ar
Re: [PATCH 1/2] file-posix: fix Linux alignment probing when EIO is returned
On Thu, Nov 03, 2022 at 10:52:43AM +0100, Kevin Wolf wrote: > Am 02.11.2022 um 03:49 hat Eric Biggers geschrieben: > > On Tue, Nov 01, 2022 at 07:27:16PM -0700, Eric Biggers wrote: > > > On Tue, Nov 01, 2022 at 03:00:30PM -0400, Stefan Hajnoczi wrote: > > > > Linux dm-crypt returns errno EIO from unaligned O_DIRECT pread(2) calls. > > > > > > Citation needed. For direct I/O to block devices, the kernel's block > > > layer > > > checks the alignment before the I/O is actually submitted to the > > > underlying > > > block device. See > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/fops.c?h=v6.1-rc3#n306 > > > > > > > Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1290 > > > > > > That "bug" seems to be based on a misunderstanding of the kernel source > > > code, > > > and not any actual testing. > > > > > > I just tested it, and the error code is EINVAL. > > > > > > > I think I see what's happening. The kernel code was broken just a few > > months > > ago, in v6.0 by the commit "block: relax direct io memory alignment" > > (https://git.kernel.org/linus/b1a000d3b8ec582d). Now the block layer lets > > DIO > > through when the user buffer is only aligned to the device's dma_alignment. > > But > > a dm-crypt device has a dma_alignment of 512 even when the crypto sector > > size > > (and thus also the logical block size) is 4096. So there is now a case > > where > > misaligned DIO can reach dm-crypt, when that shouldn't be possible. > > > > It also means that STATX_DIOALIGN will give the wrong value for > > stx_dio_mem_align in the above case, 512 instead of 4096. This is because > > STATX_DIOALIGN for block devices relies on the dma_alignment. > > In other words, STATX_DIOALIGN is unusable from the start because we > don't know whether the information it returns is actually correct? :-/ > > I guess we could still use the value returned by STATX_DIOALIGN as a > preferred value that we'll use if it survives probing, and otherwise > fall back to the same probing we've always been doing because there was > no (or no sane) way to get the information from the kernel. Yes, it seems probing is required to verify the values returned by STATX_DIOALIGN. At least until enough time passes so we can say "STATX_DIOALIGN has been correct for X years and no one is running those old kernels anymore". Stefan signature.asc Description: PGP signature
Re: [PULL v2 00/28] QAPI patches patches for 2022-10-25
Stefan Hajnoczi writes: > On Wed, 26 Oct 2022 at 14:44, Markus Armbruster wrote: >> >> The following changes since commit e750a7ace492f0b450653d4ad368a77d6f660fb8: >> >> Merge tag 'pull-9p-20221024' of https://github.com/cschoenebeck/qemu into >> staging (2022-10-24 14:27:12 -0400) >> >> are available in the Git repository at: >> >> https://repo.or.cz/qemu/armbru.git tags/pull-qapi-2022-10-25-v2 >> >> for you to fetch changes up to c0f24f8f31ca82e34ef037bfe34ef71eeecb401d: >> >> qapi: Drop temporary logic to support conversion step by step (2022-10-26 >> 20:08:52 +0200) >> >> >> QAPI patches patches for 2022-10-25 >> >> >> Markus Armbruster (28): >> docs/devel/qapi-code-gen: Update example to match current code >> qapi: Tidy up whitespace in generated code >> docs/devel/qapi-code-gen: Extend example for next commit's change >> qapi: Start to elide redundant has_FOO in generated C >> qapi tests: Elide redundant has_FOO in generated C >> qapi acpi: Elide redundant has_FOO in generated C >> qapi audio: Elide redundant has_FOO in generated C >> qapi block: Elide redundant has_FOO in generated C > > This commit breaks qemu-iotests 056 in CI. I have included > instructions for reproducing it locally below. I will drop this pull > request for now. Please note that the QEMU 7.2 soft freeze is on > Tuesday. I finally managed to debug this one. I think a v3 is best. And then we'll see whether the series can still go in, or needs to wait for the next cycle. Thanks!
[RFC PATCH] tests/avocado: improve behaviour waiting for login prompts
This attempts to deal with the problem of login prompts not being
guaranteed to be terminated with a newline. The solution to this is to
peek at the incoming data looking to see if we see an up-coming match
before we fall back to the old readline() logic. The reason to mostly
rely on readline is because I am occasionally seeing the peek stalling
despite data being there.
This seems kinda hacky and gross so I'm open to alternative approaches
and cleaner python code.
Signed-off-by: Alex Bennée
---
tests/avocado/avocado_qemu/__init__.py | 89 +-
1 file changed, 88 insertions(+), 1 deletion(-)
diff --git a/tests/avocado/avocado_qemu/__init__.py
b/tests/avocado/avocado_qemu/__init__.py
index 910f3ba1ea..d6ff68e171 100644
--- a/tests/avocado/avocado_qemu/__init__.py
+++ b/tests/avocado/avocado_qemu/__init__.py
@@ -131,6 +131,58 @@ def pick_default_qemu_bin(bin_prefix='qemu-system-',
arch=None):
return path
return None
+def _peek_ahead(console, min_match, success_message):
+"""
+peek ahead in the console stream keeping an eye out for the
+success message.
+
+Returns some message to process or None, indicating the normal
+readline should occur.
+"""
+console_logger = logging.getLogger('console')
+peek_len = 0
+retries = 0
+
+while True:
+try:
+old_peek_len = peek_len
+peek_ahead = console.peek(min_match).decode()
+peek_len = len(peek_ahead)
+
+# if we get stuck too long lets just fallback to readline
+if peek_len <= old_peek_len:
+retries += 1
+if retries > 10:
+return None
+
+# if we see a newline in the peek we can let safely bail
+# and let the normal readline() deal with it
+if peek_ahead.endswith(('\n', '\r', '\r\n')):
+return None
+
+# if we haven't seen enough for the whole message but the
+# first part matches lets just loop again
+if len(peek_ahead) < min_match and \
+ success_message[:peek_len] in peek_ahead:
+continue
+
+# if we see the whole success_message we are done, consume
+# it and pass back so we can exit to the user
+if success_message in peek_ahead:
+return console.read(peek_len).decode()
+
+# of course if we've seen enough then this line probably
+# doesn't contain what we are looking for, fallback
+if peek_len > min_match:
+return None
+
+except UnicodeDecodeError:
+console_logger.log("error in decode of peek")
+return None
+
+# we should never get here
+return None
+
def _console_interaction(test, success_message, failure_message,
send_string, keep_sending=False, vm=None):
@@ -139,17 +191,52 @@ def _console_interaction(test, success_message,
failure_message,
vm = test.vm
console = vm.console_socket.makefile(mode='rb', encoding='utf-8')
console_logger = logging.getLogger('console')
+
+# Establish the minimum number of bytes we would need to
+# potentially match against success_message
+if success_message is not None:
+min_match = len(success_message)
+else:
+min_match = 0
+
+console_logger.debug("looking for %d:(%s), sending %s (always=%s)",
+ min_match, success_message, send_string, keep_sending)
+
while True:
+msg = None
+
+# First send our string, optionally repeating the send next
+# cycle.
if send_string:
vm.console_socket.sendall(send_string.encode())
if not keep_sending:
send_string = None # send only once
+
+# If the console has no data to read we briefly
+# sleep before continuing.
+if not console.readable():
+time.sleep(0.1)
+continue
+
try:
-msg = console.readline().decode().strip()
+
+# First we shall peek ahead for a potential match to cover waiting
+# for lines without any newlines.
+if min_match > 0:
+msg = _peek_ahead(console, min_match, success_message)
+
+# otherwise we block here for a full line
+if not msg:
+msg = console.readline().decode().strip()
+
except UnicodeDecodeError:
+console_logger.debug("skipped unicode error")
msg = None
+
+# if nothing came out we continue and try again
if not msg:
continue
+
console_logger.debug(msg)
if success_message is None or success_message in msg:
break
--
2.34.1
Re: [PATCH 16/26] target/s390x: Disable conditional branch-to-next for PER
On Wed, Oct 05, 2022 at 08:44:11PM -0700, Richard Henderson wrote: > For PER, we require a conditional call to helper_per_branch > for the conditional branch. Fold the remaining optimization > into a call to helper_goto_direct, which will take care of > the remaining gbea adjustment. > > Signed-off-by: Richard Henderson > --- > target/s390x/tcg/translate.c | 13 ++--- > 1 file changed, 6 insertions(+), 7 deletions(-) Reviewed-by: Ilya Leoshkevich
Re: [PATCH 18/26] target/s390x: Split per_branch
On Wed, Oct 05, 2022 at 08:44:13PM -0700, Richard Henderson wrote:
> Split into per_branch_dest and per_branch_disp, which can be
> used for direct and indirect. In preperation for TARGET_TB_PCREL,
> call per_branch_* before indirect branches.
>
> Signed-off-by: Richard Henderson
> ---
> target/s390x/tcg/translate.c | 32 ++--
> 1 file changed, 18 insertions(+), 14 deletions(-)
>
> diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
> index 712f6d5795..bd2ee1c96e 100644
> --- a/target/s390x/tcg/translate.c
> +++ b/target/s390x/tcg/translate.c
> @@ -350,21 +350,25 @@ static inline bool per_enabled(DisasContext *s)
> #endif
> }
>
> -static void per_branch(DisasContext *s, bool to_next)
> +static void per_branch_dest(DisasContext *s, TCGv_i64 dest)
> {
> #ifndef CONFIG_USER_ONLY
> gen_psw_addr_disp(s, gbea, 0);
> +if (s->base.tb->flags & FLAG_MASK_PER) {
> +gen_helper_per_branch(cpu_env, gbea, dest);
> +}
> +#endif
> +}
>
> -if (per_enabled(s)) {
> -if (to_next) {
> -TCGv_i64 next_pc = tcg_temp_new_i64();
> -
> -gen_psw_addr_disp(s, next_pc, s->ilen);
> -gen_helper_per_branch(cpu_env, gbea, next_pc);
> -tcg_temp_free_i64(next_pc);
> -} else {
> -gen_helper_per_branch(cpu_env, gbea, psw_addr);
> -}
> +static void per_branch_disp(DisasContext *s, int64_t disp)
> +{
> +#ifndef CONFIG_USER_ONLY
> +gen_psw_addr_disp(s, gbea, 0);
> +if (s->base.tb->flags & FLAG_MASK_PER) {
> +TCGv_i64 dest = tcg_temp_new_i64();
> +gen_psw_addr_disp(s, dest, disp);
> +gen_helper_per_branch(cpu_env, gbea, dest);
> +tcg_temp_free_i64(dest);
> }
> #endif
> }
...
Nit: maybe use per_enabled(s) instead of testing the mask manually?
Reviewed-by: Ilya Leoshkevich
Re: [PATCH] linux-user: implement execveat
You're right, that's a much better approach. New patch coming up shortly.
Re: [PULL v2 00/82] pci,pc,virtio: features, tests, fixes, cleanups
On Thu, Nov 03, 2022 at 09:29:56AM -0400, Stefan Hajnoczi wrote: > On Thu, 3 Nov 2022 at 08:14, Michael S. Tsirkin wrote: > > On Wed, Nov 02, 2022 at 03:47:43PM -0400, Stefan Hajnoczi wrote: > > > On Wed, Nov 02, 2022 at 12:02:14PM -0400, Michael S. Tsirkin wrote: > > > > Changes from v1: > > > > > > > > Applied and squashed fixes by Igor, Lei He, Hesham Almatary for > > > > bugs that tripped up the pipeline. > > > > Updated expected files for core-count test. > > > > > > Several "make check" CI failures have occurred. They look like they are > > > related. Here is one (see the URLs at the bottom of this email for more > > > details): > > > > > > 17/106 ERROR:../tests/qtest/qos-test.c:191:subprocess_run_one_test: child > > > process > > > (/arm/virt/virtio-mmio/virtio-bus/virtio-net-device/virtio-net/virtio-net-tests/vhost-user/flags-mismatch/subprocess > > > [8609]) failed unexpectedly ERROR > > > 17/106 qemu:qtest+qtest-arm / qtest-arm/qos-test > > > ERROR 31.44s killed by signal 6 SIGABRT > > > >>> G_TEST_DBUS_DAEMON=/builds/qemu-project/qemu/tests/dbus-vmstate-daemon.sh > > > >>> MALLOC_PERTURB_=49 QTEST_QEMU_IMG=./qemu-img > > > >>> QTEST_QEMU_BINARY=./qemu-system-arm > > > >>> QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon > > > >>> /builds/qemu-project/qemu/build/tests/qtest/qos-test --tap -k > > > ― ✀ > > > ― > > > stderr: > > > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > > > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument > > > (22) > > > qemu-system-arm: Failed to set msg fds. > > > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument > > > (22) > > > qemu-system-arm: -chardev > > > socket,id=chr-reconnect,path=/tmp/vhost-test-6PT2U1/reconnect.sock,server=on: > > > info: QEMU waiting for connection on: > > > disconnected:unix:/tmp/vhost-test-6PT2U1/reconnect.sock,server=on > > > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > > > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument > > > (22) > > > qemu-system-arm: Failed to set msg fds. > > > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument > > > (22) > > > qemu-system-arm: -chardev > > > socket,id=chr-connect-fail,path=/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on: > > > info: QEMU waiting for connection on: > > > disconnected:unix:/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on > > > qemu-system-arm: -netdev > > > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: Failed to read > > > msg header. Read 0 instead of 12. Original request 1. > > > qemu-system-arm: -netdev > > > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: > > > vhost_backend_init failed: Protocol error > > > qemu-system-arm: -netdev > > > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: failed to init > > > vhost_net for queue 0 > > > qemu-system-arm: -netdev > > > vhost-user,id=hs0,chardev=chr-connect-fail,vhostforce=on: info: QEMU > > > waiting for connection on: > > > disconnected:unix:/tmp/vhost-test-H8G7U1/connect-fail.sock,server=on > > > qemu-system-arm: Failed to write msg. Wrote -1 instead of 20. > > > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument > > > (22) > > > qemu-system-arm: Failed to set msg fds. > > > qemu-system-arm: vhost VQ 1 ring restore failed: -22: Invalid argument > > > (22) > > > qemu-system-arm: -chardev > > > socket,id=chr-flags-mismatch,path=/tmp/vhost-test-94UYU1/flags-mismatch.sock,server=on: > > > info: QEMU waiting for connection on: > > > disconnected:unix:/tmp/vhost-test-94UYU1/flags-mismatch.sock,server=on > > > qemu-system-arm: Failed to write msg. Wrote -1 instead of 52. > > > qemu-system-arm: vhost_set_mem_table failed: Invalid argument (22) > > > qemu-system-arm: Failed to set msg fds. > > > qemu-system-arm: vhost VQ 0 ring restore failed: -22: Invalid argument > > > (22) > > > UndefinedBehaviorSanitizer:DEADLYSIGNAL > > > ==8618==ERROR: UndefinedBehaviorSanitizer: SEGV on unknown address > > > 0x (pc 0x55e34deccab0 bp 0x sp 0x7ffc94894710 > > > T8618) > > > ==8618==The signal is caused by a READ memory access. > > > ==8618==Hint: address points to the zero page. > > > #0 0x55e34deccab0 in ldl_he_p > > > /builds/qemu-project/qemu/include/qemu/bswap.h:301:5 > > > #1 0x55e34deccab0 in ldn_he_p > > > /builds/qemu-project/qemu/include/qemu/bswap.h:440:1 > > > #2 0x55e34deccab0 in flatview_write_continue > > > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2824:19 > > > #3 0x55e34dec9f21 in flatview_write > > > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2867:12 > > > #4 0x55e34dec9f21 in address_space_write > > > /builds/qemu-project/qemu/build/../softmmu/physmem.c:2963:18 > > > #5 0x55e34decace7 in address_space_unmap > > > /builds/qemu-pro
[PATCH v2] linux-user: implement execveat
References: https://gitlab.com/qemu-project/qemu/-/issues/1007
Signed-off-by: Drew DeVault
---
linux-user/syscall.c | 204 +++
1 file changed, 111 insertions(+), 93 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f55cdebee5..57f0b2f0e8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -633,7 +633,12 @@ safe_syscall4(pid_t, wait4, pid_t, pid, int *, status,
int, options, \
#endif
safe_syscall5(int, waitid, idtype_t, idtype, id_t, id, siginfo_t *, infop, \
int, options, struct rusage *, rusage)
+#if defined(TARGET_NR_execveat)
+safe_syscall5(int, execveat, int, dirfd, const char *, filename,
+char **, argv, char **, envp, int, flags)
+#else
safe_syscall3(int, execve, const char *, filename, char **, argv, char **,
envp)
+#endif
#if defined(TARGET_NR_select) || defined(TARGET_NR__newselect) || \
defined(TARGET_NR_pselect6) || defined(TARGET_NR_pselect6_time64)
safe_syscall6(int, pselect6, int, nfds, fd_set *, readfds, fd_set *, writefds,
\
@@ -8281,6 +8286,107 @@ static int do_openat(CPUArchState *cpu_env, int dirfd,
const char *pathname, int
return safe_openat(dirfd, path(pathname), flags, mode);
}
+static int do_execveat(CPUArchState *cpu_env, int dirfd, abi_long pathname,
abi_long guest_argp, abi_long guest_envp, int flags)
+{
+int ret;
+char **argp, **envp;
+int argc, envc;
+abi_ulong gp;
+abi_ulong addr;
+char **q;
+void *p;
+
+argc = 0;
+
+for (gp = guest_argp; gp; gp += sizeof(abi_ulong)) {
+if (get_user_ual(addr, gp))
+return -TARGET_EFAULT;
+if (!addr)
+break;
+argc++;
+}
+envc = 0;
+for (gp = guest_envp; gp; gp += sizeof(abi_ulong)) {
+if (get_user_ual(addr, gp))
+return -TARGET_EFAULT;
+if (!addr)
+break;
+envc++;
+}
+
+argp = g_new0(char *, argc + 1);
+envp = g_new0(char *, envc + 1);
+
+for (gp = guest_argp, q = argp; gp;
+ gp += sizeof(abi_ulong), q++) {
+if (get_user_ual(addr, gp))
+goto execve_efault;
+if (!addr)
+break;
+if (!(*q = lock_user_string(addr)))
+goto execve_efault;
+}
+*q = NULL;
+
+for (gp = guest_envp, q = envp; gp;
+ gp += sizeof(abi_ulong), q++) {
+if (get_user_ual(addr, gp))
+goto execve_efault;
+if (!addr)
+break;
+if (!(*q = lock_user_string(addr)))
+goto execve_efault;
+}
+*q = NULL;
+
+/* Although execve() is not an interruptible syscall it is
+ * a special case where we must use the safe_syscall wrapper:
+ * if we allow a signal to happen before we make the host
+ * syscall then we will 'lose' it, because at the point of
+ * execve the process leaves QEMU's control. So we use the
+ * safe syscall wrapper to ensure that we either take the
+ * signal as a guest signal, or else it does not happen
+ * before the execve completes and makes it the other
+ * program's problem.
+ */
+if (!(p = lock_user_string(pathname)))
+goto execve_efault;
+
+#if defined(TARGET_NR_execveat)
+ret = get_errno(safe_execveat(dirfd, p, argp, envp, flags));
+#else
+assert(dirfd == AT_FDCWD && flags == 0);
+ret = get_errno(safe_execve(p, argp, envp));
+#endif
+
+unlock_user(p, pathname, 0);
+
+goto execve_end;
+
+execve_efault:
+ret = -TARGET_EFAULT;
+
+execve_end:
+for (gp = guest_argp, q = argp; *q;
+ gp += sizeof(abi_ulong), q++) {
+if (get_user_ual(addr, gp)
+|| !addr)
+break;
+unlock_user(*q, addr, 0);
+}
+for (gp = guest_envp, q = envp; *q;
+ gp += sizeof(abi_ulong), q++) {
+if (get_user_ual(addr, gp)
+|| !addr)
+break;
+unlock_user(*q, addr, 0);
+}
+
+g_free(argp);
+g_free(envp);
+return ret;
+}
+
#define TIMER_MAGIC 0x0caf
#define TIMER_MAGIC_MASK 0x
@@ -8748,101 +8854,13 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int
num, abi_long arg1,
ret = get_errno(unlinkat(arg1, p, arg3));
unlock_user(p, arg2, 0);
return ret;
+#endif
+#if defined(TARGET_NR_execveat)
+case TARGET_NR_execveat:
+return do_execveat(cpu_env, arg1, arg2, arg3, arg4, arg5);
#endif
case TARGET_NR_execve:
-{
-char **argp, **envp;
-int argc, envc;
-abi_ulong gp;
-abi_ulong guest_argp;
-abi_ulong guest_envp;
-abi_ulong addr;
-char **q;
-
-argc = 0;
-guest_argp = arg2;
-for (gp = guest_argp; gp; gp += sizeof(abi_ulong)) {
-if (get_user_ual(addr, gp))
-return -TARGET_EFAULT;
-if (!addr)
-break;
-argc++;
-
Re: [PATCH v6 0/3] ppc/e500: Add support for eSDHC
On 11/3/22 09:51, BALATON Zoltan wrote:
On Wed, 2 Nov 2022, Daniel Henrique Barboza wrote:
On 11/1/22 19:29, Philippe Mathieu-Daudé wrote:
This is a respin of Bernhard's v4 with Freescale eSDHC implemented
as an 'UNIMP' region. See v4 cover here:
https://lore.kernel.org/qemu-devel/20221018210146.193159-1-shen...@gmail.com/
Since v5:
- Rebased (ppc-next merged)
- Properly handle big-endian
Since v4:
- Do not rename ESDHC_* definitions to USDHC_*
- Do not modify SDHCIState structure
Supersedes: <20221031115402.91912-1-phi...@linaro.org>
Queued in gitlab.com/danielhb/qemu/tree/ppc-8.0 (since we missed the
freeze for 7.2).
Could you please always use ppc-next to queue patches for the next upcoming
version and ppc-7.2 for the current version? Unless this makes your workflow
harder in which case ignore this but the reason I ask is because then it's
enough for me to only track ppc-next if I need to rebase patches on that and
don't have to add a new branch at every release (unless I have some patches to
rebase on it during a freeze but that's less likely than rebasing on your
queued patches for the next release xo using version for the current branch and
keep next for the future versions makes more sense to me).
Note that doing "ppc-7.2" for the current release and ppc-next for the
next release will not prevent you from adding a new branch at every
release, e.g. for the next release you would need to add a ppc-8.0
branch.
'ppc-next' is used like a standard, a way of telling 'this is the next
pull request that is going upstream'. Can we change it? Sure, but if the
idea is to avoid new branches every new release then I suggest the
following:
- ppc-next: keep it as is
- ppc-next-release/ppc-future: branch that will host any code for the next
release during the code freeze window. Note that this branch will become
'ppc-next' when the new release cycle begins
This would avoid changing everyone's workflow with the current ppc-next
usage, while also standardize a branch for the future release patches
during freeze.
BTW, checkpatch complained about this line being too long (83 chars):
3/3 Checking commit bc7b8cc88560 (hw/ppc/e500: Add Freescale eSDHC to e500plat)
WARNING: line over 80 characters
#150: FILE: hw/ppc/e500.c:1024:
+ pmc->ccsrbar_base +
MPC85XX_ESDHC_REGS_OFFSET,
The code except is this:
if (pmc->has_esdhc) {
create_unimplemented_device("esdhc",
pmc->ccsrbar_base +
MPC85XX_ESDHC_REGS_OFFSET,
MPC85XX_ESDHC_REGS_SIZE);
To get rid of the warning we would need to make a python-esque identation (line
break after "(" ) or create a new variable to hold the sum. Both seems overkill
so I'll ignore the warning. Phil is welcome to re-send if he thinks it's worth
it.
Or you could break indentation and not start at the ( but 3 chars back. I.e.:
create_unimplemented_device("esdhc",
pmc->ccsrbar_base + MPC85XX_ESDHC_REGS_OFFSET,
MPC85XX_ESDHC_REGS_SIZE);
But I think it can be just ignored in this case.
And I'll follow it up with my usual plea in these cases: can we move the line
size warning to 100 chars? For QEMU 8.0? Pretty please?
I think the consensus was to keep 80 columns if possible, this is good becuase
you can open more files side by side (although it does not match well with the
long _ naming convention of glib and qemu) but we have a distinction between
checkpatch warning and error in line length. I think it will give error at 90
chars but as long as it's just warns that means: fix it if you can but in rare
cases if it's more readable with a slightly longer line then it is still
acceptable. I think that's the case here, splitting the line would be less
readable than a few chars longer line.
Yeah I know that we can usually ignore these warnings. I keep bringing
this up because it's weird to keep bothering with 80 chars per line when
people are using 28" flat screen monitors, multiple screen desktops
and so on.
Thanks,
Daniel
Regards,
BALATON Zoltan
Re: [PULL v2 00/82] pci,pc,virtio: features, tests, fixes, cleanups
> To pull this image: > $ docker pull registry.gitlab.com/qemu-project/qemu/fedora:latest Actually the URL is: $ docker pull registry.gitlab.com/qemu-project/qemu/qemu/fedora:latest > (or to be sure to pull the very same:) > $ docker pull > registry.gitlab.com/qemu-project/qemu/fedora:d6d20c1c6aede3a652eb01b781530cc10392de2764503c84f9bf4eb1d7a89d26 Same here, registry.gitlab.com/qemu-project/qemu/qemu/fedora:d6d20c1c6aede3a652eb01b781530cc10392de2764503c84f9bf4eb1d7a89d26 See https://gitlab.com/qemu-project/qemu/container_registry/1215910
Re: [PULL v2 00/82] pci,pc,virtio: features, tests, fixes, cleanups
gitlab-runner can run locally with minimal setup: https://bagong.gitlab.io/posts/run-gitlab-ci-locally/ I haven't tried it yet, but that seems like the most reliable (and easiest) way to reproduce the CI environment. Stefan
Re: [PULL v2 00/82] pci,pc,virtio: features, tests, fixes, cleanups
On Thu, Nov 03, 2022 at 11:49:21AM -0400, Stefan Hajnoczi wrote: > gitlab-runner can run locally with minimal setup: > https://bagong.gitlab.io/posts/run-gitlab-ci-locally/ > > I haven't tried it yet, but that seems like the most reliable (and > easiest) way to reproduce the CI environment. > > Stefan How does one pass in variables do you know? Environment? -- MST
