Re: [Bug 1926497] Re: dp83932 stops working after a short while
On Wed, Apr 28, 2021 at 11:31 PM Jeff <1926...@bugs.launchpad.net> wrote: > > It looks like using > https://cdimage.debian.org/cdimage/ports/snapshots/2021-04-17/debian-10.0.0 > -m68k-NETINST-1.iso instead fixes the issue. Perhaps the instruction on > https://wiki.qemu.org/Documentation/Platforms/m68k should be updated. > > -- > You received this bug notification because you are a member of qemu- > devel-ml, which is subscribed to QEMU. > https://bugs.launchpad.net/bugs/1926497 > > Title: > dp83932 stops working after a short while > > Status in QEMU: > New > > Bug description: > Following the instructions here > https://wiki.qemu.org/Documentation/Platforms/m68k I was able to > successfully install debian. However, running apt-get update stalls > after the first 1-2MB. > > root@debian:~# apt-get update > Get:1 http://ftp.ports.debian.org/debian-ports sid InRelease [55.3 kB] > Ign:1 http://ftp.ports.debian.org/debian-ports sid InRelease > Get:2 http://ftp.ports.debian.org/debian-ports sid/main all Packages [8,735 > kB] > 18% [2 Packages 2,155 kB/8,735 kB 25%] > > After running apt-get update. I don't seem to be able to send any > packets anymore. ping host lookups fail and a subsequent apt-get > update makes no progress. > > I'm launching qemu with: > > qemu-system-m68k -boot c \ >-M q800 -serial none -serial mon:stdio -m 1000M \ >-net nic,model=dp83932 -net user \ >-append "root=/dev/sda2 rw console=ttyS0 console=tty" \ >-kernel vmlinux-4.16.0-1-m68k \ >-initrd initrd.img-4.16.0-1-m68k \ >-drive file=m68k-deb10.qcow2,format=qcow2 \ >-nographic > > I see this with qemu v6.0.0-rc5 > > To manage notifications about this bug go to: > https://bugs.launchpad.net/qemu/+bug/1926497/+subscriptions I've updated the page to include: Please note that the instructions below use kernel versions that might have been superseded by newer ones on the most recent installation cd images! Also, during installation on hard disk image the update process might install a newer kernel. Always make sure to extract the latest kernel and initrd.gz from your hard disk image after installation or update and replace the kernel names in the examples below with what is currently installed.
Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine
Stefan Hajnoczi writes: > On Wed, Apr 28, 2021 at 04:18:17PM +0200, Markus Armbruster wrote: >> Stefan Hajnoczi writes: [...] >> > The approach in this patch is okay but we should keep in mind it only >> > solves piix3-ide. ISA provides a non-qdev backdoor API and there may be >> > more instances of this type of bug. >> > >> > A qdev fix would address the root cause and make it possible to drop the >> > backdoor API, but that's probably too much work for little benefit. >> >> What do you mean by backdoor API? Global @isabus? > > Yes. It's also strange that isa_get_irq(ISADevice *dev, unsigned isairq) > accepts dev = NULL as a valid argument. @isabus is static in hw/isa/isa-bus.c. Uses: * Limit isa_bus_new() to one ISA bus. Arbitrary restriction; multiple ISA buses could work with suitable memory mapping and IRQ wiring. "Single ISA bus" assumptions could of course hide elsewhere in the code. * Implied argument to isa_get_irq(), isa_register_ioport(), isa_register_portio_list(), isa_address_space(), isa_address_space_io(). isa_get_irq() asserts that a non-null @dev is a child of @isabus. This means we don't actually need @isabus, except when @dev is null. I suspect two separate functions would be cleaner: one taking an ISABus * argument, and a wrapper taking an ISADevice * argument. isa_address_space() and isa_address_space_io() work the same, less the assertion. isa_register_ioport() and isa_register_portio_list() take a non-null @dev argument. They don't actually need @isabus. To eliminate global @isabus, we need to fix up the callers passing null @dev. Clean solution: plumb the ISABus returned by isa_bus_new() to the call sites. Where that's impractical, we can also get it from QOM, like build_dsdt_microvm() does.
Re: [PATCH v4 10/12] qtest/qmp-cmd-test: Make test build-independent from accelerator
Philippe Mathieu-Daudé writes:
> Now than we can probe if the TCG accelerator is available
> at runtime with a QMP command, do it once at the beginning
> and only register the tests we can run.
> We can then replace the #ifdef'ry by a runtime check.
>
> Suggested-by: Paolo Bonzini
> Signed-off-by: Philippe Mathieu-Daudé
Please read the last remark first. The other ones are detail; feel free
to skip them until we're done with the last one.
> ---
> tests/qtest/qmp-cmd-test.c | 18 ++
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
> index c98b78d0339..8902d2f169f 100644
> --- a/tests/qtest/qmp-cmd-test.c
> +++ b/tests/qtest/qmp-cmd-test.c
> @@ -21,19 +21,24 @@ const char common_args[] = "-nodefaults -machine none";
>
> /* Query smoke tests */
>
> +static bool tcg_accel_available;
> +
> static int query_error_class(const char *cmd)
> {
> -static struct {
> +static const struct {
> const char *cmd;
> int err_class;
> +/*
> + * Pointer to boolean.
Let's not paraphrase code in comments.
> + * If non-NULL and value is %true, the error class is skipped.
Suggest "When it points to true, the test case is skipped", and ...
> + */
> +bool *skip_err_class;
... name this just @skip. Or maybe @skip_ptr, because fails[i].skip
reads as if the test case was to be skipped.
> } fails[] = {
> /* Success depends on build configuration: */
Note the headline ^^^
> #ifndef CONFIG_SPICE
> { "query-spice", ERROR_CLASS_COMMAND_NOT_FOUND },
> #endif
> -#ifndef CONFIG_TCG
> -{ "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND },
> -#endif
> +{ "query-replay", ERROR_CLASS_COMMAND_NOT_FOUND,
> &tcg_accel_available },
No longer fits under the headline. Move it under its own headline,
perhaps
/* Success depends on dynamic state as reported by other queries */
> #ifndef CONFIG_VNC
> { "query-vnc", ERROR_CLASS_GENERIC_ERROR },
> { "query-vnc-servers", ERROR_CLASS_GENERIC_ERROR },
> @@ -51,6 +56,9 @@ static int query_error_class(const char *cmd)
> int i;
>
> for (i = 0; fails[i].cmd; i++) {
> +if (fails[i].skip_err_class && *fails[i].skip_err_class) {
> +continue;
> +}
> if (!strcmp(cmd, fails[i].cmd)) {
> return fails[i].err_class;
> }
> @@ -334,6 +342,8 @@ int main(int argc, char *argv[])
> QmpSchema schema;
> int ret;
>
> +tcg_accel_available = qtest_has_accel("tcg");
> +
When does tcg_accel_available differ from defined(CONFIG_TCG)?
> g_test_init(&argc, &argv, NULL);
>
> qmp_schema_init(&schema);
RE: [PATCH] Hexagon (target/hexagon) remove unused functions
> -Original Message-
> From: Philippe Mathieu-Daudé On
> Behalf Of Philippe Mathieu-Daudé
> Sent: Wednesday, April 28, 2021 11:49 PM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: richard.hender...@linaro.org; a...@rev.ng; Brian Cain
>
> Subject: Re: [PATCH] Hexagon (target/hexagon) remove unused functions
>
> Hi Taylor,
>
> On 4/29/21 5:32 AM, Taylor Simpson wrote:
> > Remove gen_read_reg and gen_set_byte
> >
> > Reported-by: Richard Henderson
> > Signed-off-by: Taylor Simpson
> > ---
>
> To help git-tools (and reviewers), please use the 'Based-on' tag
> the next time you send a patch depending on another one:
> Based-on: <1617930474-31979-1-git-send-email-tsimp...@quicinc.com>
>
> > target/hexagon/genptr.c | 11 ---
> > 1 file changed, 11 deletions(-)
> >
> > diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> > index 55c7cd8..f93f895 100644
> > --- a/target/hexagon/genptr.c
> > +++ b/target/hexagon/genptr.c
> > @@ -28,12 +28,6 @@
> > #undef QEMU_GENERATE
> > #include "gen_tcg.h"
> >
> > -static inline TCGv gen_read_reg(TCGv result, int num)
> > -{
> > -tcg_gen_mov_tl(result, hex_gpr[num]);
> > -return result;
> > -}
>
> But what about:
>
> target/hexagon/macros.h:26:#define READ_REG(dest, NUM)
> gen_read_reg(dest, NUM)
I should remove this to avoid confusion.
> target/hexagon/macros.h:29:#define READ_REG(NUM)
> (env->gpr[(NUM)])
> target/hexagon/macros.h:360:#define fREAD_LR()
> (READ_REG(HEX_REG_LR))
> target/hexagon/macros.h:366:#define fREAD_SP()
> (READ_REG(HEX_REG_SP))
> target/hexagon/macros.h:367:#define fREAD_LC0
> (READ_REG(HEX_REG_LC0))
> target/hexagon/macros.h:368:#define fREAD_LC1
> (READ_REG(HEX_REG_LC1))
> target/hexagon/macros.h:369:#define fREAD_SA0
> (READ_REG(HEX_REG_SA0))
> target/hexagon/macros.h:370:#define fREAD_SA1
> (READ_REG(HEX_REG_SA1))
> target/hexagon/macros.h:371:#define fREAD_FP()
> (READ_REG(HEX_REG_FP))
> target/hexagon/macros.h:375:(insn->extension_valid ? 0 :
> READ_REG(HEX_REG_GP))
> target/hexagon/macros.h:377:#define fREAD_GP()
> READ_REG(HEX_REG_GP)
> target/hexagon/macros.h:379:#define fREAD_PC()
> (READ_REG(HEX_REG_PC))
> target/hexagon/macros.h:577:#define fGET_FRAMEKEY()
> READ_REG(HEX_REG_FRAMEKEY)
These are not guarded by QEMU_GENERATE, so they are needed and reference the
other version of READ_REG
> and:
>
> target/hexagon/genptr.c:37:static inline TCGv gen_read_preg(TCGv pred,
> uint8_t num)
> target/hexagon/macros.h:27:#define READ_PREG(dest, NUM)
> gen_read_preg(dest, (NUM))
This is needed. It reads a predicate register. The gen_read_reg functions
reads a general purpose register.
> I'd rather send a "!fixup Hexagon (target/hexagon) circular addressing"
> patch (so Richard squashes it there) with:
Richard said he could cherry pick a single patch.
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg05968.html
>
> -- >8 --
> diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
> index b726c3b7917..bf0e5ae92bb 100644
> --- a/target/hexagon/macros.h
> +++ b/target/hexagon/macros.h
> @@ -22,16 +22,11 @@
> #include "hex_regs.h"
> #include "reg_fields.h"
>
> -#ifdef QEMU_GENERATE
> -#define READ_REG(dest, NUM) gen_read_reg(dest, NUM)
> -#define READ_PREG(dest, NUM) gen_read_preg(dest, (NUM))
> -#else
> #define READ_REG(NUM)(env->gpr[(NUM)])
> #define READ_PREG(NUM) (env->pred[NUM])
>
> #define WRITE_RREG(NUM, VAL) log_reg_write(env, NUM, VAL, slot)
> #define WRITE_PREG(NUM, VAL) log_pred_write(env, NUM, VAL)
> -#endif
>
> #define PCALIGN 4
> #define PCALIGN_MASK (PCALIGN - 1)
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> index 55c7cd86a4e..42f25f6f462 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -28,18 +28,6 @@
> #undef QEMU_GENERATE
> #include "gen_tcg.h"
>
> -static inline TCGv gen_read_reg(TCGv result, int num)
> -{
> -tcg_gen_mov_tl(result, hex_gpr[num]);
> -return result;
> -}
> -
> -static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
> -{
> -tcg_gen_mov_tl(pred, hex_pred[num]);
> -return pred;
> -}
> -
> static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int
> slot)
> {
> TCGv zero = tcg_const_tl(0);
> @@ -319,11 +307,6 @@ static inline void gen_set_half_i64(int N, TCGv_i64
> result, TCGv src)
> tcg_temp_free_i64(src64);
> }
>
> -static inline void gen_set_byte(int N, TCGv result, TCGv src)
> -{
> -tcg_gen_deposit_tl(result, result, src, N * 8, 8);
> -}
> -
> static void gen_set_byte_i64(int N, TCGv_i64 result, TCGv src)
> {
> TCGv_i64 src64 = tcg_temp_new_i64();
> ---
>
> NB: untested :)
>
> Regards,
>
> Phil.
Re: [PATCH] Hexagon (target/hexagon) remove unused functions
On Thu, Apr 29, 2021 at 6:49 AM Philippe Mathieu-Daudé wrote: > > Hi Taylor, > > On 4/29/21 5:32 AM, Taylor Simpson wrote: > > Remove gen_read_reg and gen_set_byte > > > > Reported-by: Richard Henderson > > Signed-off-by: Taylor Simpson > > --- > > To help git-tools (and reviewers), please use the 'Based-on' tag > the next time you send a patch depending on another one: > Based-on: <1617930474-31979-1-git-send-email-tsimp...@quicinc.com> Sorry I forgot to link the explanation: https://wiki.qemu.org/Contribute/SubmitAPatch#Base_patches_against_current_git_master > > > target/hexagon/genptr.c | 11 --- > > 1 file changed, 11 deletions(-)
Re: [PATCH] meson: change buildtype when debug_info=no
On 4/28/21 9:55 PM, Joelle van Dyne wrote: > Meson defaults builds to 'debugoptimized' which adds '-g -O2' > to CFLAGS. If the user specifies '--disable-debug-info' we > should instead build with 'release' which does not emit any > debug info. > > Signed-off-by: Joelle van Dyne > --- > configure | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/configure b/configure > index 4f374b4889..5c3568cbc3 100755 > --- a/configure > +++ b/configure > @@ -6398,6 +6398,7 @@ NINJA=$ninja $meson setup \ > --sysconfdir "$sysconfdir" \ > --localedir "$localedir" \ > --localstatedir "$local_statedir" \ > +--buildtype $(if test "$debug_info" = yes; then echo > "debugoptimized"; else echo "release"; fi) \ NAck. You are changing the default (which is 'debug') to 'release'. This should be at least mentioned in the commit description, but I don't think this is what we want here. 'release' enables -O3, which is certainly not supported. The 'debug' profile is what we have been and are testing. I'd be OK if you had used "debugoptimized else debug". The mainstream project would rather use 'debug'/'debugoptimized', or 'minsize', which are already tested. We might consider allowing forks to use 'plain' profile eventually. But the 'release' type is an unsupported landmine IMHO. If you want to use something else, it should be an explicit argument to ./configure, then you are on your own IMO. Regards, Phil.
Re: [PATCH] Hexagon (target/hexagon) remove unused functions
Hi Taylor,
On 4/29/21 5:32 AM, Taylor Simpson wrote:
> Remove gen_read_reg and gen_set_byte
>
> Reported-by: Richard Henderson
> Signed-off-by: Taylor Simpson
> ---
To help git-tools (and reviewers), please use the 'Based-on' tag
the next time you send a patch depending on another one:
Based-on: <1617930474-31979-1-git-send-email-tsimp...@quicinc.com>
> target/hexagon/genptr.c | 11 ---
> 1 file changed, 11 deletions(-)
>
> diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
> index 55c7cd8..f93f895 100644
> --- a/target/hexagon/genptr.c
> +++ b/target/hexagon/genptr.c
> @@ -28,12 +28,6 @@
> #undef QEMU_GENERATE
> #include "gen_tcg.h"
>
> -static inline TCGv gen_read_reg(TCGv result, int num)
> -{
> -tcg_gen_mov_tl(result, hex_gpr[num]);
> -return result;
> -}
But what about:
target/hexagon/macros.h:26:#define READ_REG(dest, NUM)
gen_read_reg(dest, NUM)
target/hexagon/macros.h:29:#define READ_REG(NUM)
(env->gpr[(NUM)])
target/hexagon/macros.h:360:#define fREAD_LR() (READ_REG(HEX_REG_LR))
target/hexagon/macros.h:366:#define fREAD_SP() (READ_REG(HEX_REG_SP))
target/hexagon/macros.h:367:#define fREAD_LC0 (READ_REG(HEX_REG_LC0))
target/hexagon/macros.h:368:#define fREAD_LC1 (READ_REG(HEX_REG_LC1))
target/hexagon/macros.h:369:#define fREAD_SA0 (READ_REG(HEX_REG_SA0))
target/hexagon/macros.h:370:#define fREAD_SA1 (READ_REG(HEX_REG_SA1))
target/hexagon/macros.h:371:#define fREAD_FP() (READ_REG(HEX_REG_FP))
target/hexagon/macros.h:375:(insn->extension_valid ? 0 :
READ_REG(HEX_REG_GP))
target/hexagon/macros.h:377:#define fREAD_GP() READ_REG(HEX_REG_GP)
target/hexagon/macros.h:379:#define fREAD_PC() (READ_REG(HEX_REG_PC))
target/hexagon/macros.h:577:#define fGET_FRAMEKEY()
READ_REG(HEX_REG_FRAMEKEY)
and:
target/hexagon/genptr.c:37:static inline TCGv gen_read_preg(TCGv pred,
uint8_t num)
target/hexagon/macros.h:27:#define READ_PREG(dest, NUM)
gen_read_preg(dest, (NUM))
I'd rather send a "!fixup Hexagon (target/hexagon) circular addressing"
patch (so Richard squashes it there) with:
-- >8 --
diff --git a/target/hexagon/macros.h b/target/hexagon/macros.h
index b726c3b7917..bf0e5ae92bb 100644
--- a/target/hexagon/macros.h
+++ b/target/hexagon/macros.h
@@ -22,16 +22,11 @@
#include "hex_regs.h"
#include "reg_fields.h"
-#ifdef QEMU_GENERATE
-#define READ_REG(dest, NUM) gen_read_reg(dest, NUM)
-#define READ_PREG(dest, NUM) gen_read_preg(dest, (NUM))
-#else
#define READ_REG(NUM)(env->gpr[(NUM)])
#define READ_PREG(NUM) (env->pred[NUM])
#define WRITE_RREG(NUM, VAL) log_reg_write(env, NUM, VAL, slot)
#define WRITE_PREG(NUM, VAL) log_pred_write(env, NUM, VAL)
-#endif
#define PCALIGN 4
#define PCALIGN_MASK (PCALIGN - 1)
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 55c7cd86a4e..42f25f6f462 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -28,18 +28,6 @@
#undef QEMU_GENERATE
#include "gen_tcg.h"
-static inline TCGv gen_read_reg(TCGv result, int num)
-{
-tcg_gen_mov_tl(result, hex_gpr[num]);
-return result;
-}
-
-static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
-{
-tcg_gen_mov_tl(pred, hex_pred[num]);
-return pred;
-}
-
static inline void gen_log_predicated_reg_write(int rnum, TCGv val, int
slot)
{
TCGv zero = tcg_const_tl(0);
@@ -319,11 +307,6 @@ static inline void gen_set_half_i64(int N, TCGv_i64
result, TCGv src)
tcg_temp_free_i64(src64);
}
-static inline void gen_set_byte(int N, TCGv result, TCGv src)
-{
-tcg_gen_deposit_tl(result, result, src, N * 8, 8);
-}
-
static void gen_set_byte_i64(int N, TCGv_i64 result, TCGv src)
{
TCGv_i64 src64 = tcg_temp_new_i64();
---
NB: untested :)
Regards,
Phil.
Re: [RFC PATCH 3/4] target/ppc: Move SPR generation to separate file
On Wed, Apr 28, 2021 at 02:47:37PM +, Bruno Piazera Larsen wrote: > > > > This move is required to enable building without TCG. > > > > All the logic related to registering SPRs specific to > > > > some architectures or machines has been hidden in this > > > > new file. > > > > > > Hm... I thought we ended up deciding to keep the gen_spr_ > > > functions in translate_init.c.inc (cpu_init.c). I don't strongly favour > > > one way or the other, but one alternative would be to just rename the > > > gen_spr_ functions to add_sprs_ or even > > > register__sprs and leave them where they are. > > > > Right. I think renaming these away from gen_*() is a good idea, to > > avoid confusion with the other gen_*() functions which, well, generate > > TCG code. > > > > I don't think there's a lot of value in moving them out from > > translate_init. Honestly the more useful way to break up > > translate_init would be by CPU family, rather than by SPR vs. non-SPR > > setup > > Right, ok. I thought we agreed to separate, but I can't remember the reason. > I guess less is more in this case, so I won't create the new file. > As for separating by CPU family: sounds good for a later cleanup, but I don't > think it's a priority right now. That's fair. > I'll work on that renaming, I agree its a good idea. Great. A head's up, there are also patches in preparation which add 64-bit instruction support. Those are also replacing some of our existing instruction decode logic with the new somewhat generic decodetree system. That seems to be replacing the gen_*() convention for TCG op generating functions with trans_*() for the new style decodetree op generating functions. Nonetheless, it's worth renaming the SPR construction functions to something that won't collide with either the old or new convention. However, since you're working in adjacent areas, one or both of you are likely to have to do some rebasing and conflict fixing along the way. > > > > The idea of this final patch is to hide all SPR generation from > > > > translate_init, but in an effort to simplify the RFC the 4 > > > > functions for not_implemented SPRs were created. They'll be > > > > substituted by gen_spr__misc in reusable ways for the > > > > final patch. > > > > > > I'd expect this patch to be just a big removal of gen_spr* from > > > translate_init.c.inc and their addition into spr_common.c. So any other > > > prep work should come in separate patches ealier in the > > > series. Specifically, at least one patch for the macro work and another > > > for the refactoring of open coded spr_register calls into gen_spr_* > > > functions. > > > > Seconded. > > Ok. Should it be 2 commits (refactoring, then macro) or 3 commits (renaming, > vscr_init, then macro)? I guess also that since I won't move stuff, I don't > need to fix the vscr_init, but it's no big deal at this point. 3 please. In general, lean towards more, simpler commits. > > > If you're only adding this now, it means the previous patch is > > > broken. As a general rule you should make sure every patch works. I know > > > that for the RFC things might be a bit broken temporarily and that is ok > > > but it is always a good idea to make sure every individual change is in > > > the correct patch at least. > > yeah... sorry about that. I'm correcting all these problems. > > > > > +/*/ > > > > +/* SPR definitions and registration */ > > > > + > > > > +#ifdef CONFIG_TCG > > > > +#ifdef CONFIG_USER_ONLY > > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, > > > > \ > > > > + oea_read, oea_write, one_reg_id, > > > > initial_value) \ > > > > +_spr_register(env, num, name, uea_read, uea_write, initial_value) > > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, > > > > \ > > > > +oea_read, oea_write, hea_read, hea_write, > > > > \ > > > > +one_reg_id, initial_value) > > > > \ > > > > +_spr_register(env, num, name, uea_read, uea_write, initial_value) > > > > +#else /* CONFIG_USER_ONLY */ > > > > +#if !defined(CONFIG_KVM) > > > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, > > > > \ > > > > + oea_read, oea_write, one_reg_id, > > > > initial_value) \ > > > > +_spr_register(env, num, name, uea_read, uea_write, > > > > \ > > > > + oea_read, oea_write, oea_read, oea_write, > > > > initial_value) > > > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, > > > > \ > > > > +oea_read, oea_write, hea_read, hea_write, > > > > \ > > > > +one_reg_id, initial_value) > > > > \ > > > > +_spr_register(env
[PATCH v4 3/3] nvdimm: Enable sync-dax device property for nvdimm
The patch adds the 'sync-dax' property to the nvdimm device.
When the sync-dax is 'direct' indicates the backend is synchronous DAX
capable and no explicit flush requests are required. When the mode is
set to 'writeback' it indicates the backend is not synhronous DAX
capable and explicit flushes to Hypervisor are required.
On PPC where the flush requests from guest can be honoured by the qemu,
the 'writeback' mode is supported and set as the default. The device
tree property "hcall-flush-required" is added to the nvdimm node which
makes the guest to issue H_SCM_FLUSH hcalls to request for flushes
explicitly. This would be the default behaviour without sync-dax
property set for the nvdimm device. For old pSeries machine, the
default is 'unsafe'.
For non-PPC platforms, the mode is set to 'unsafe' as the default.
Signed-off-by: Shivaprasad G Bhat
---
hw/arm/virt.c | 28 +++--
hw/i386/pc.c| 28 +++--
hw/mem/nvdimm.c | 52 +++
hw/ppc/spapr.c | 10 +
hw/ppc/spapr_nvdimm.c | 39 +++
include/hw/mem/nvdimm.h | 11 ++
include/hw/ppc/spapr.h |1 +
qapi/common.json| 20 ++
8 files changed, 179 insertions(+), 10 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 9f01d9041b..f32e3e4010 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2358,6 +2358,27 @@ static const CPUArchIdList
*virt_possible_cpu_arch_ids(MachineState *ms)
return ms->possible_cpus;
}
+static bool virt_nvdimm_validate(const MachineState *ms, NVDIMMDevice *nvdimm,
+ Error **errp)
+{
+NvdimmSyncModes sync;
+
+if (!ms->nvdimms_state->is_enabled) {
+error_setg(errp, "nvdimm is not enabled: add 'nvdimm=on' to '-M'");
+return false;
+}
+
+sync = object_property_get_enum(OBJECT(nvdimm), NVDIMM_SYNC_DAX_PROP,
+"NvdimmSyncModes", &error_abort);
+if (sync == NVDIMM_SYNC_MODES_WRITEBACK) {
+error_setg(errp, "NVDIMM device " NVDIMM_SYNC_DAX_PROP
+ "=%s mode unsupported", NvdimmSyncModes_str(sync));
+return false;
+}
+
+return true;
+}
+
static void virt_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
@@ -2376,9 +2397,10 @@ static void virt_memory_pre_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
return;
}
-if (is_nvdimm && !ms->nvdimms_state->is_enabled) {
-error_setg(errp, "nvdimm is not enabled: add 'nvdimm=on' to '-M'");
-return;
+if (is_nvdimm) {
+if (!virt_nvdimm_validate(ms, NVDIMM(dev), errp)) {
+return;
+}
}
pc_dimm_pre_plug(PC_DIMM(dev), MACHINE(hotplug_dev), NULL, errp);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 8a84b25a03..2d5151462c 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1211,6 +1211,27 @@ void pc_i8259_create(ISABus *isa_bus, qemu_irq
*i8259_irqs)
g_free(i8259);
}
+static bool pc_nvdimm_validate(const MachineState *ms, NVDIMMDevice *nvdimm,
+ Error **errp)
+{
+NvdimmSyncModes sync;
+
+if (!ms->nvdimms_state->is_enabled) {
+error_setg(errp, "nvdimm is not enabled: add 'nvdimm=on' to '-M'");
+return false;
+}
+
+sync = object_property_get_enum(OBJECT(nvdimm), NVDIMM_SYNC_DAX_PROP,
+"NvdimmSyncModes", &error_abort);
+if (sync == NVDIMM_SYNC_MODES_WRITEBACK) {
+error_setg(errp, "NVDIMM device " NVDIMM_SYNC_DAX_PROP
+ "=%s mode unsupported", NvdimmSyncModes_str(sync));
+return false;
+}
+
+return true;
+}
+
static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
Error **errp)
{
@@ -1233,9 +1254,10 @@ static void pc_memory_pre_plug(HotplugHandler
*hotplug_dev, DeviceState *dev,
return;
}
-if (is_nvdimm && !ms->nvdimms_state->is_enabled) {
-error_setg(errp, "nvdimm is not enabled: missing 'nvdimm' in '-M'");
-return;
+if (is_nvdimm) {
+if (!pc_nvdimm_validate(ms, NVDIMM(dev), errp)) {
+return;
+}
}
hotplug_handler_pre_plug(x86ms->acpi_dev, dev, &local_err);
diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 7397b67156..56b4527362 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -96,6 +96,19 @@ static void nvdimm_set_uuid(Object *obj, Visitor *v, const
char *name,
g_free(value);
}
+static int nvdimm_get_sync_mode(Object *obj, Error **errp G_GNUC_UNUSED)
+{
+NVDIMMDevice *nvdimm = NVDIMM(obj);
+
+return nvdimm->sync_dax;
+}
+
+static void nvdimm_set_sync_mode(Object *obj, int mode, Error **errp)
+{
+NVDIMMDevice *nvdimm = NVDIMM(obj);
+
+nvdimm->sync_dax = mode;
+}
static void nvdimm_ini
[PATCH v4 2/3] spapr: nvdimm: Implement H_SCM_FLUSH hcall
The patch adds support for the SCM flush hcall for the nvdimm devices.
To be available for exploitation by guest through the next patch.
The hcall expects the semantics such that the flush to return
with H_BUSY when the operation is expected to take longer time along
with a continue_token. The hcall to be called again providing the
continue_token to get the status. So, all fresh requsts are put into
a 'pending' list and flush worker is submitted to the thread pool.
The thread pool completion callbacks move the requests to 'completed'
list, which are cleaned up after reporting to guest in subsequent
hcalls to get the status.
The semantics makes it necessary to preserve the continue_tokens
and their return status across migrations. So, the completed
flush states are forwarded to the destination and the pending
ones are restarted at the destination in post_load. The necessary
nvdimm flush specific vmstate structures are added to the spapr
machine vmstate.
Signed-off-by: Shivaprasad G Bhat
---
hw/ppc/spapr.c|6 +
hw/ppc/spapr_nvdimm.c | 234 +
include/hw/ppc/spapr.h| 10 ++
include/hw/ppc/spapr_nvdimm.h | 13 ++
4 files changed, 262 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index e4be00b732..80957f9188 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1607,6 +1607,8 @@ static void spapr_machine_reset(MachineState *machine)
spapr->ov5_cas = spapr_ovec_clone(spapr->ov5);
}
+spapr_nvdimm_finish_flushes(spapr);
+
/* DRC reset may cause a device to be unplugged. This will cause troubles
* if this device is used by another device (eg, a running vhost backend
* will crash QEMU if the DIMM holding the vring goes away). To avoid such
@@ -2003,6 +2005,7 @@ static const VMStateDescription vmstate_spapr = {
&vmstate_spapr_cap_ccf_assist,
&vmstate_spapr_cap_fwnmi,
&vmstate_spapr_fwnmi,
+&vmstate_spapr_nvdimm_states,
NULL
}
};
@@ -2997,6 +3000,9 @@ static void spapr_machine_init(MachineState *machine)
}
qemu_cond_init(&spapr->fwnmi_machine_check_interlock_cond);
+
+QLIST_INIT(&spapr->pending_flush_states);
+QLIST_INIT(&spapr->completed_flush_states);
}
#define DEFAULT_KVM_TYPE "auto"
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index 8cf3fb2ffb..77eb7e1293 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -22,6 +22,7 @@
* THE SOFTWARE.
*/
#include "qemu/osdep.h"
+#include "qemu/cutils.h"
#include "qapi/error.h"
#include "hw/ppc/spapr_drc.h"
#include "hw/ppc/spapr_nvdimm.h"
@@ -30,6 +31,7 @@
#include "hw/ppc/fdt.h"
#include "qemu/range.h"
#include "hw/ppc/spapr_numa.h"
+#include "block/thread-pool.h"
/*
* The nvdimm size should be aligned to SCM block size.
@@ -371,6 +373,237 @@ static target_ulong h_scm_bind_mem(PowerPCCPU *cpu,
SpaprMachineState *spapr,
return H_SUCCESS;
}
+static uint64_t flush_token;
+
+static int flush_worker_cb(void *opaque)
+{
+int ret = H_SUCCESS;
+SpaprNVDIMMDeviceFlushState *state = opaque;
+
+/* flush raw backing image */
+if (qemu_fdatasync(state->backend_fd) < 0) {
+error_report("papr_scm: Could not sync nvdimm to backend file: %s",
+ strerror(errno));
+ret = H_HARDWARE;
+}
+
+return ret;
+}
+
+static void spapr_nvdimm_flush_completion_cb(void *opaque, int hcall_ret)
+{
+SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+SpaprNVDIMMDeviceFlushState *state = opaque;
+
+state->hcall_ret = hcall_ret;
+QLIST_REMOVE(state, node);
+QLIST_INSERT_HEAD(&spapr->completed_flush_states, state, node);
+}
+
+static const VMStateDescription vmstate_spapr_nvdimm_flush_state = {
+ .name = "spapr_nvdimm_flush_state",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT64(continue_token, SpaprNVDIMMDeviceFlushState),
+ VMSTATE_INT64(hcall_ret, SpaprNVDIMMDeviceFlushState),
+ VMSTATE_UINT32(drcidx, SpaprNVDIMMDeviceFlushState),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static bool spapr_nvdimm_states_needed(void *opaque)
+{
+ SpaprMachineState *spapr = (SpaprMachineState *)opaque;
+
+ return (!QLIST_EMPTY(&spapr->pending_flush_states) ||
+ !QLIST_EMPTY(&spapr->completed_flush_states));
+}
+
+static int spapr_nvdimm_post_load(void *opaque, int version_id)
+{
+SpaprMachineState *spapr = (SpaprMachineState *)opaque;
+SpaprNVDIMMDeviceFlushState *state, *next;
+PCDIMMDevice *dimm;
+HostMemoryBackend *backend = NULL;
+ThreadPool *pool = aio_get_thread_pool(qemu_get_aio_context());
+SpaprDrc *drc;
+
+QLIST_FOREACH_SAFE(state, &spapr->completed_flush_states, node, next) {
+if (flush_token < state->continue_token) {
+flush_token = state->continue_token;
+}
+}
+
+QLIST_FOREACH_SAFE(sta
[PATCH v4 1/3] spapr: nvdimm: Forward declare and move the definitions
The subsequent patches add definitions which tend to
get the compilation to cyclic dependency. So, prepare
with forward declarations, move the defitions and clean up.
Signed-off-by: Shivaprasad G Bhat
---
hw/ppc/spapr_nvdimm.c | 12
include/hw/ppc/spapr_nvdimm.h | 14 ++
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index b46c36917c..8cf3fb2ffb 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -31,6 +31,18 @@
#include "qemu/range.h"
#include "hw/ppc/spapr_numa.h"
+/*
+ * The nvdimm size should be aligned to SCM block size.
+ * The SCM block size should be aligned to SPAPR_MEMORY_BLOCK_SIZE
+ * inorder to have SCM regions not to overlap with dimm memory regions.
+ * The SCM devices can have variable block sizes. For now, fixing the
+ * block size to the minimum value.
+ */
+#define SPAPR_MINIMUM_SCM_BLOCK_SIZE SPAPR_MEMORY_BLOCK_SIZE
+
+/* Have an explicit check for alignment */
+QEMU_BUILD_BUG_ON(SPAPR_MINIMUM_SCM_BLOCK_SIZE % SPAPR_MEMORY_BLOCK_SIZE);
+
bool spapr_nvdimm_validate(HotplugHandler *hotplug_dev, NVDIMMDevice *nvdimm,
uint64_t size, Error **errp)
{
diff --git a/include/hw/ppc/spapr_nvdimm.h b/include/hw/ppc/spapr_nvdimm.h
index 73be250e2a..764f999f54 100644
--- a/include/hw/ppc/spapr_nvdimm.h
+++ b/include/hw/ppc/spapr_nvdimm.h
@@ -11,19 +11,9 @@
#define HW_SPAPR_NVDIMM_H
#include "hw/mem/nvdimm.h"
-#include "hw/ppc/spapr.h"
-/*
- * The nvdimm size should be aligned to SCM block size.
- * The SCM block size should be aligned to SPAPR_MEMORY_BLOCK_SIZE
- * inorder to have SCM regions not to overlap with dimm memory regions.
- * The SCM devices can have variable block sizes. For now, fixing the
- * block size to the minimum value.
- */
-#define SPAPR_MINIMUM_SCM_BLOCK_SIZE SPAPR_MEMORY_BLOCK_SIZE
-
-/* Have an explicit check for alignment */
-QEMU_BUILD_BUG_ON(SPAPR_MINIMUM_SCM_BLOCK_SIZE % SPAPR_MEMORY_BLOCK_SIZE);
+typedef struct SpaprDrc SpaprDrc;
+typedef struct SpaprMachineState SpaprMachineState;
int spapr_pmem_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
void *fdt, int *fdt_start_offset, Error **errp);
[PATCH v4 0/3] nvdimm: Enable sync-dax property for nvdimm
The nvdimm devices are expected to ensure write persistence during power failure kind of scenarios. The libpmem has architecture specific instructions like dcbf on POWER to flush the cache data to backend nvdimm device during normal writes followed by explicit flushes if the backend devices are not synchronous DAX capable. Qemu - virtual nvdimm devices are memory mapped. The dcbf in the guest and the subsequent flush doesn't traslate to actual flush to the backend file on the host in case of file backed v-nvdimms. This is addressed by virtio-pmem in case of x86_64 by making explicit flushes translating to fsync at qemu. On SPAPR, the issue is addressed by adding a new hcall to request for an explicit flush from the guest ndctl driver when the backend nvdimm cannot ensure write persistence with dcbf alone. So, the approach here is to convey when the hcall flush is required in a device tree property. The guest makes the hcall when the property is found, instead of relying on dcbf. A new device property sync-dax is added to the nvdimm device. When the sync-dax is 'writeback'(default for PPC), device property "hcall-flush-required" is set, and the guest makes hcall H_SCM_FLUSH requesting for an explicit flush. sync-dax is "unsafe" on all other platforms(x86, ARM) and old pseries machines prior to 5.2 on PPC. sync-dax="writeback" on ARM and x86_64 is prevented now as the flush semantics are unimplemented. When the backend file is actually synchronous DAX capable and no explicit flushes are required, the sync-dax mode 'direct' is to be used. The below demonstration shows the map_sync behavior with sync-dax writeback & direct. (https://github.com/avocado-framework-tests/avocado-misc-tests/blob/master/memory/ndctl.py.data/map_sync.c) The pmem0 is from nvdimm with With sync-dax=direct, and pmem1 is from nvdimm with syn-dax=writeback, mounted as /dev/pmem0 on /mnt1 type xfs (rw,relatime,attr2,dax=always,inode64,logbufs=8,logbsize=32k,noquota) /dev/pmem1 on /mnt2 type xfs (rw,relatime,attr2,dax=always,inode64,logbufs=8,logbsize=32k,noquota) [root@atest-guest ~]# ./mapsync /mnt1/newfile > When sync-dax=unsafe/direct [root@atest-guest ~]# ./mapsync /mnt2/newfile > when sync-dax=writeback Failed to mmap with Operation not supported The first patch does the header file cleanup necessary for the subsequent ones. Second patch implements the hcall, adds the necessary vmstate properties to spapr machine structure for carrying the hcall status during save-restore. The nature of the hcall being asynchronus, the patch uses aio utilities to offload the flush. The third patch adds the 'sync-dax' device property and enables the device tree property for the guest to utilise the hcall. The kernel changes to exploit this hcall is at https://github.com/linuxppc/linux/commit/75b7c05ebf9026.patch --- v3 - https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg07916.html Changes from v3: - Fixed the forward declaration coding guideline violations in 1st patch. - Removed the code waiting for the flushes to complete during migration, instead restart the flush worker on destination qemu in post load. - Got rid of the randomization of the flush tokens, using simple counter. - Got rid of the redundant flush state lock, relying on the BQL now. - Handling the memory-backend-ram usage - Changed the sync-dax symantics from on/off to 'unsafe','writeback' and 'direct'. Added prevention code using 'writeback' on arm and x86_64. - Fixed all the miscellaneous comments. v2 - https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg07031.html Changes from v2: - Using the thread pool based approach as suggested - Moved the async hcall handling code to spapr_nvdimm.c along with some simplifications - Added vmstate to preserve the hcall status during save-restore along with pre_save handler code to complete all ongoning flushes. - Added hw_compat magic for sync-dax 'on' on previous machines. - Miscellanious minor fixes. v1 - https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg06330.html Changes from v1 - Fixed a missed-out unlock - using QLIST_FOREACH instead of QLIST_FOREACH_SAFE while generating token Shivaprasad G Bhat (3): spapr: nvdimm: Forward declare and move the definitions spapr: nvdimm: Implement H_SCM_FLUSH hcall nvdimm: Enable sync-dax device property for nvdimm hw/arm/virt.c | 28 hw/i386/pc.c | 28 hw/mem/nvdimm.c | 52 +++ hw/ppc/spapr.c| 16 ++ hw/ppc/spapr_nvdimm.c | 285 + include/hw/mem/nvdimm.h | 11 ++ include/hw/ppc/spapr.h| 11 +- include/hw/ppc/spapr_nvdimm.h | 27 ++-- qapi/common.json | 20 +++ 9 files changed, 455 insertions(+), 23 deletions(-) -- Signature
Re: [PATCH v6 0/4] Add support for ipv6 host forwarding
Ping. On Wed, Apr 14, 2021 at 8:39 PM Doug Evans wrote: > This patchset takes the original patch from Maxim, > https://www.mail-archive.com/qemu-devel@nongnu.org/msg569573.html > and updates it. > > Option hostfwd is extended to support ipv6 addresses. > Commands hostfwd_add, hostfwd_remove are extended as well. > > The libslirp part of the patch has been committed upstream, > and is now in qemu. See patch 1/4. > > Changes from v5: > > 1/4 slirp: Advance libslirp submodule to current master > NOTE TO REVIEWERS: It may be a better use of everyone's time if a > maintainer takes on advancing QEMU's libslirp to libslirp's master. > Beyond that, I really don't know what to do except submit this patch as > is currently provided. > > 2/4: util/qemu-sockets.c: Split host:port parsing out of inet_parse > > Also split out parsing of ipv4=on|off, ipv6=on|off > > 3/4: net/slirp.c: Refactor address parsing > > Use InetSocketAddress and getaddrinfo(). > Use new libslirp calls: slirp_remove_hostxfwd, slirp_add_hostxfwd. > > 4/4: net: Extend host forwarding to support IPv6 > > Recognize ipv4=,ipv6= options. > > Note: v5's 3/5 "Recognize []:port (empty ipv6 address)" has been deleted: > the churn on this patch series needs to be reduced. > This change is not required, and can easily be done in a later patch. > > Changes from v4: > > 1/5 slirp: Advance libslirp submodule to add ipv6 host-forward support > NOTE TO REVIEWERS: I need some hand-holding to know what The Right > way to submit this particular patch is. > > - no change > > 2/5 util/qemu-sockets.c: Split host:port parsing out of inet_parse > > - move recognition of "[]:port" to separate patch > - allow passing NULL for ip_v6 > - fix some formatting issues > > 3/5 inet_parse_host_and_addr: Recognize []:port (empty ipv6 address) > > - new in this patchset revision > > 4/5 net/slirp.c: Refactor address parsing > > - was 3/4 in v4 > - fix some formatting issues > > 5/5 net: Extend host forwarding to support IPv6 > > - was 4/4 in v4 > - fix some formatting issues > > Changes from v3: > > 1/4 slirp: Advance libslirp submodule to add ipv6 host-forward support > > - pick up latest libslirp patch to reject ipv6 addr-any for guest address > - libslirp currently only provides a stateless DHCPv6 server, which means > it can't know in advance what the guest's IP address is, and thus > cannot do the "addr-any -> guest ip address" translation that is done > for ipv4 > > 2/4 util/qemu-sockets.c: Split host:port parsing out of inet_parse > > - this patch is new in v4 > - provides new utility: inet_parse_host_and_port, updates inet_parse > to use it > > 3/4 net/slirp.c: Refactor address parsing > > - this patch renamed from 2/3 to 3/4 > - call inet_parse_host_and_port from util/qemu-sockets.c > - added tests/acceptance/hostfwd.py > > 4/4 net: Extend host forwarding to support IPv6 > > - this patch renamed from 3/3 to 4/4 > - ipv6 support added to existing hostfwd option, commands > - instead of creating new ipv6 option, commands > - added tests to tests/acceptance/hostfwd.py > > Changes from v2: > - split out libslirp commit > - clarify spelling of ipv6 addresses in docs > - tighten parsing of ipv6 addresses > > Change from v1: > - libslirp part is now upstream > - net/slirp.c changes split into two pieces (refactor, add ipv6) > - added docs > > Doug Evans (4): > slirp: Advance libslirp submodule to add ipv6 host-forward support > util/qemu-sockets.c: Split host:port parsing out of inet_parse > net/slirp.c: Refactor address parsing > net: Extend host forwarding to support IPv6 > > hmp-commands.hx | 18 ++- > include/qemu/sockets.h | 5 + > net/slirp.c | 236 ++-- > slirp | 2 +- > tests/acceptance/hostfwd.py | 185 > util/qemu-sockets.c | 82 + > 6 files changed, 436 insertions(+), 92 deletions(-) > create mode 100644 tests/acceptance/hostfwd.py > > -- > 2.31.1.295.g9ea45b61b8-goog > >
[PATCH] Hexagon (target/hexagon) remove unused functions
Remove gen_read_reg and gen_set_byte
Reported-by: Richard Henderson
Signed-off-by: Taylor Simpson
---
target/hexagon/genptr.c | 11 ---
1 file changed, 11 deletions(-)
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 55c7cd8..f93f895 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -28,12 +28,6 @@
#undef QEMU_GENERATE
#include "gen_tcg.h"
-static inline TCGv gen_read_reg(TCGv result, int num)
-{
-tcg_gen_mov_tl(result, hex_gpr[num]);
-return result;
-}
-
static inline TCGv gen_read_preg(TCGv pred, uint8_t num)
{
tcg_gen_mov_tl(pred, hex_pred[num]);
@@ -319,11 +313,6 @@ static inline void gen_set_half_i64(int N, TCGv_i64
result, TCGv src)
tcg_temp_free_i64(src64);
}
-static inline void gen_set_byte(int N, TCGv result, TCGv src)
-{
-tcg_gen_deposit_tl(result, result, src, N * 8, 8);
-}
-
static void gen_set_byte_i64(int N, TCGv_i64 result, TCGv src)
{
TCGv_i64 src64 = tcg_temp_new_i64();
--
2.7.4
RE: sysemu SMP scheduling
> -Original Message- > From: Brian Cain > Sent: Wednesday, April 28, 2021 10:06 PM > To: qemu-devel@nongnu.org > Cc: Richard Henderson ; Taylor Simpson > ; Michael Lambert ; > Manning, Sid > Subject: sysemu SMP scheduling > > For some hexagon use cases, we would prefer to have finer grained scheduling > among multiple guest cores/threads. We haven't been able to determine > exactly what kind of scheduling algorithm is operating in the baseline case. > If > the current hw thread is ready-to-run and is spinning over a tight loop that > never hits any exceptions, would it ever yield to another thread after so-many > iterations/TBs executed? Or perhaps since we're executing translated blocks > there's just no yield opportunity available? > > We came up with a design for this finer-grained scheduling feature, but are > re- > examining whether or not it should be necessary and if it is necessary, > whether > it should have been designed like so. We haven't seen a similar example in > other targets, so we'd love to get feedback on the approach. > > In the TranslatorOps .tb_stop() we generate code like so: > > if the current count of ready-to-run threads >= 2: > tb_count++ > if tb_count > THRESHOLD: > gen_exception(EXCP_YIELD); > tb_count = 0 > gen exit_tb Hmm, I omitted some critical details of this algorithm. tb_stop, corrected: if the current count of ready-to-run threads >= 2: consecutive_tb_count++ if consecutive_tb_count > THRESHOLD: gen_exception(EXCP_YIELD); consecutive_tb_count = 0 gen exit_tb last_cpu_id = this_cpu_id tb_start generates code like this: if this_cpu_id != last_cpu_id: consecutive_tb_count = 0 > - "current count of ready-to-run threads" is based on the values in the CPU > state. When entering a wait/halt mode, we set the appropriate state and call > cpu_stop_current(). > - Is EXCP_YIELD an appropriate mechanism for this feature? It seems like > maybe it has no special handling, but any exception can trigger a yield? > > -Brian
Re: [PATCH v6 1/9] hw: Add check for queue number
On Tue, Apr 27, 2021 at 1:39 PM Jason Wang wrote:
>
>
> 在 2021/4/27 上午11:39, Cindy Lu 写道:
> > In order to support configure interrupt. we will use queue number -1
> > as configure interrupt
> > since all these device are not support the configure interrupt
> > So we will add an check here, if the idx is -1, the function
> > will return;
>
>
> The title is confusing since the change is specific for the guest notifiers.
>
> A better one would be "virtio: guest notifier support for config interrupt"
>
sure, will fix this
>
> >
> > Signed-off-by: Cindy Lu
> > ---
> > hw/display/vhost-user-gpu.c| 8 ++--
> > hw/net/virtio-net.c| 10 +++---
> > hw/virtio/vhost-user-fs.c | 11 +++
> > hw/virtio/vhost-vsock-common.c | 8 ++--
> > hw/virtio/virtio-crypto.c | 8 ++--
> > 5 files changed, 32 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/display/vhost-user-gpu.c b/hw/display/vhost-user-gpu.c
> > index 51f1747c4a..d8e26cedf1 100644
> > --- a/hw/display/vhost-user-gpu.c
> > +++ b/hw/display/vhost-user-gpu.c
> > @@ -490,7 +490,9 @@ static bool
> > vhost_user_gpu_guest_notifier_pending(VirtIODevice *vdev, int idx)
> > {
> > VhostUserGPU *g = VHOST_USER_GPU(vdev);
> > -
> > +if (idx == -1) {
>
>
> Let's introduce a macro for this instead of the magic number.
>
> Thanks
>
>
sure will fix this
> > +return false;
> > +}
> > return vhost_virtqueue_pending(&g->vhost->dev, idx);
> > }
> >
> > @@ -498,7 +500,9 @@ static void
> > vhost_user_gpu_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
> > {
> > VhostUserGPU *g = VHOST_USER_GPU(vdev);
> > -
> > +if (idx == -1) {
> > +return;
> > +}
> > vhost_virtqueue_mask(&g->vhost->dev, vdev, idx, mask);
> > }
> >
> > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> > index 9179013ac4..78ccaa228c 100644
> > --- a/hw/net/virtio-net.c
> > +++ b/hw/net/virtio-net.c
> > @@ -3060,7 +3060,10 @@ static bool
> > virtio_net_guest_notifier_pending(VirtIODevice *vdev, int idx)
> > VirtIONet *n = VIRTIO_NET(vdev);
> > NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
> > assert(n->vhost_started);
> > -return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx);
> > +if (idx != -1) {
> > +return vhost_net_virtqueue_pending(get_vhost_net(nc->peer), idx);
> > +}
> > +return false;
> > }
> >
> > static void virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
> > @@ -3069,8 +3072,9 @@ static void
> > virtio_net_guest_notifier_mask(VirtIODevice *vdev, int idx,
> > VirtIONet *n = VIRTIO_NET(vdev);
> > NetClientState *nc = qemu_get_subqueue(n->nic, vq2q(idx));
> > assert(n->vhost_started);
> > -vhost_net_virtqueue_mask(get_vhost_net(nc->peer),
> > - vdev, idx, mask);
> > +if (idx != -1) {
> > +vhost_net_virtqueue_mask(get_vhost_net(nc->peer), vdev, idx, mask);
> > + }
> > }
> >
> > static void virtio_net_set_config_size(VirtIONet *n, uint64_t
> > host_features)
> > diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c
> > index 1bc5d03a00..37424c2193 100644
> > --- a/hw/virtio/vhost-user-fs.c
> > +++ b/hw/virtio/vhost-user-fs.c
> > @@ -142,18 +142,21 @@ static void vuf_handle_output(VirtIODevice *vdev,
> > VirtQueue *vq)
> >*/
> > }
> >
> > -static void vuf_guest_notifier_mask(VirtIODevice *vdev, int idx,
> > -bool mask)
> > +static void vuf_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask)
> > {
> > VHostUserFS *fs = VHOST_USER_FS(vdev);
> > -
> > +if (idx == -1) {
> > +return;
> > +}
> > vhost_virtqueue_mask(&fs->vhost_dev, vdev, idx, mask);
> > }
> >
> > static bool vuf_guest_notifier_pending(VirtIODevice *vdev, int idx)
> > {
> > VHostUserFS *fs = VHOST_USER_FS(vdev);
> > -
> > +if (idx == -1) {
> > +return false;
> > +}
> > return vhost_virtqueue_pending(&fs->vhost_dev, idx);
> > }
> >
> > diff --git a/hw/virtio/vhost-vsock-common.c b/hw/virtio/vhost-vsock-common.c
> > index 5b2ebf3496..0adf823d37 100644
> > --- a/hw/virtio/vhost-vsock-common.c
> > +++ b/hw/virtio/vhost-vsock-common.c
> > @@ -100,7 +100,9 @@ static void
> > vhost_vsock_common_guest_notifier_mask(VirtIODevice *vdev, int idx,
> > bool mask)
> > {
> > VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
> > -
> > +if (idx == -1) {
> > +return;
> > +}
> > vhost_virtqueue_mask(&vvc->vhost_dev, vdev, idx, mask);
> > }
> >
> > @@ -108,7 +110,9 @@ static bool
> > vhost_vsock_common_guest_notifier_pending(VirtIODevice *vdev,
> > int idx)
> > {
> > VHostVSockCommon *vvc = VHOST_VSOCK_COMMON(vdev);
> > -
> > +if (idx == -1) {
> > +return false;
> > +}
> > retu
Re: [PATCH v6 7/9] virtio-pci: add support for configure interrupt
On Tue, Apr 27, 2021 at 3:12 PM Jason Wang wrote:
>
>
> 在 2021/4/27 上午11:39, Cindy Lu 写道:
> > Add support for configure interrupt, use kvm_irqfd_assign and set the
> > gsi to kernel. When the configure notifier was eventfd_signal by host
> > kernel, this will finally inject an msix interrupt to guest
> > ---
> > hw/virtio/virtio-pci.c | 186 ++---
> > 1 file changed, 120 insertions(+), 66 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> > index 2b7e6cc0d9..07d28dd367 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -664,12 +664,10 @@ static uint32_t virtio_read_config(PCIDevice *pci_dev,
> > }
> >
> > static int kvm_virtio_pci_vq_vector_use(VirtIOPCIProxy *proxy,
> > -unsigned int queue_no,
> > unsigned int vector)
> > {
> > VirtIOIRQFD *irqfd = &proxy->vector_irqfd[vector];
> > int ret;
> > -
>
>
> Unnecessary changes.
>
will fix this
>
> > if (irqfd->users == 0) {
> > ret = kvm_irqchip_add_msi_route(kvm_state, vector,
> > &proxy->pci_dev);
> > if (ret < 0) {
> > @@ -708,93 +706,120 @@ static void
> > kvm_virtio_pci_irqfd_release(VirtIOPCIProxy *proxy,
> > ret = kvm_irqchip_remove_irqfd_notifier_gsi(kvm_state, n,
> > irqfd->virq);
> > assert(ret == 0);
> > }
> > -
>
>
> So did here.
>
will fix this
>
> > -static int kvm_virtio_pci_vector_use(VirtIOPCIProxy *proxy, int nvqs)
> > + static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, int queue_no,
> > + EventNotifier **n, unsigned int
> > *vector)
>
>
> The indentation looks not correct.
>
>
> > {
> > PCIDevice *dev = &proxy->pci_dev;
> > VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
> > -VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> > -unsigned int vector;
> > -int ret, queue_no;
> > VirtQueue *vq;
> > -EventNotifier *n;
> > -for (queue_no = 0; queue_no < nvqs; queue_no++) {
> > +
> > +if (queue_no == -1) {
> > +*n = virtio_get_config_notifier(vdev);
> > +*vector = vdev->config_vector;
> > +} else {
> > if (!virtio_queue_get_num(vdev, queue_no)) {
> > -break;
> > -}
> > -vector = virtio_queue_vector(vdev, queue_no);
> > -if (vector >= msix_nr_vectors_allocated(dev)) {
> > -continue;
> > -}
> > -ret = kvm_virtio_pci_vq_vector_use(proxy, queue_no, vector);
> > -if (ret < 0) {
> > -goto undo;
> > -}
> > -/* If guest supports masking, set up irqfd now.
> > - * Otherwise, delay until unmasked in the frontend.
> > - */
> > -if (vdev->use_guest_notifier_mask && k->guest_notifier_mask) {
> > -vq = virtio_get_queue(vdev, queue_no);
> > -n = virtio_queue_get_guest_notifier(vq);
> > -ret = kvm_virtio_pci_irqfd_use(proxy, n, vector);
> > -if (ret < 0) {
> > -kvm_virtio_pci_vq_vector_release(proxy, vector);
> > -goto undo;
> > -}
> > +return -1;
> > }
> > +*vector = virtio_queue_vector(vdev, queue_no);
> > +vq = virtio_get_queue(vdev, queue_no);
> > +*n = virtio_queue_get_guest_notifier(vq);
> > +}
> > +if (*vector >= msix_nr_vectors_allocated(dev)) {
> > +return -1;
> > }
> > return 0;
> > +}
> >
> > +static int kvm_virtio_pci_vector_use_one(VirtIOPCIProxy *proxy, int
> > queue_no)
> > +{
>
>
> Let's use separate patch for the introducing of
> kvm_virtio_pci_vector_user/release_one().
>
> And then do the config interrupt support on top.
>
Sure, will fix this
> Thanks
>
sysemu SMP scheduling
For some hexagon use cases, we would prefer to have finer grained scheduling among multiple guest cores/threads. We haven't been able to determine exactly what kind of scheduling algorithm is operating in the baseline case. If the current hw thread is ready-to-run and is spinning over a tight loop that never hits any exceptions, would it ever yield to another thread after so-many iterations/TBs executed? Or perhaps since we're executing translated blocks there's just no yield opportunity available? We came up with a design for this finer-grained scheduling feature, but are re-examining whether or not it should be necessary and if it is necessary, whether it should have been designed like so. We haven't seen a similar example in other targets, so we'd love to get feedback on the approach. In the TranslatorOps .tb_stop() we generate code like so: if the current count of ready-to-run threads >= 2: tb_count++ if tb_count > THRESHOLD: gen_exception(EXCP_YIELD); tb_count = 0 gen exit_tb - "current count of ready-to-run threads" is based on the values in the CPU state. When entering a wait/halt mode, we set the appropriate state and call cpu_stop_current(). - Is EXCP_YIELD an appropriate mechanism for this feature? It seems like maybe it has no special handling, but any exception can trigger a yield? -Brian
[PATCH] meson: Set implicit_include_directories to false
Without this, libvixl cannot be compiled with macOS 11.3 SDK due to
include file name conflict (usr/include/c++/v1/version conflicts with
VERSION).
Signed-off-by: Katsuhiro Ueno
---
meson.build | 1 +
1 file changed, 1 insertion(+)
diff --git a/meson.build b/meson.build
index c6f4b0cf5e..d007bff8c3 100644
--- a/meson.build
+++ b/meson.build
@@ -2129,6 +2129,7 @@ common_all = common_ss.apply(config_all, strict: false)
common_all = static_library('common',
build_by_default: false,
sources: common_all.sources() + genh,
+implicit_include_directories: false,
dependencies: common_all.dependencies(),
name_suffix: 'fa')
--
2.30.1 (Apple Git-130)
Re: [RFC PATCH v2 6/6] hw/arm/virt: Replace smp_parse with one that prefers cores
Hi Drew, On 2021/4/28 18:13, Andrew Jones wrote: On Wed, Apr 28, 2021 at 05:36:43PM +0800, wangyanan (Y) wrote: On 2021/4/27 22:58, Andrew Jones wrote: On Tue, Apr 13, 2021 at 04:07:45PM +0800, Yanan Wang wrote: From: Andrew Jones The virt machine type has never used the CPU topology parameters, other than number of online CPUs and max CPUs. When choosing how to allocate those CPUs the default has been to assume cores. In preparation for using the other CPU topology parameters let's use an smp_parse that prefers cores over sockets. We can also enforce the topology matches max_cpus check because we have no legacy to preserve. Signed-off-by: Andrew Jones Signed-off-by: Yanan Wang --- hw/arm/virt.c | 76 +++ 1 file changed, 76 insertions(+) Thanks, this patch matches [1]. Of course, I've always considered this patch to be something of an RFC, though. Is there any harm in defaulting to sockets over cores? If not, I wonder if we shouldn't just leave the default as it is to avoid a mach-virt specific smp parser. The "no topology" compat variable will keep existing machine types from switching from cores to sockets, so we don't need to worry about that. [1] https://github.com/rhdrjones/qemu/commit/c0670b1bccb4d08c7cf7c6957cc8878a2af131dd For CPU topology population, ARM64 kernel will firstly try to parse ACPI PPTT table and then DT in function init_cpu_topology(), if failed it will rely on the MPIDR value in function store_cpu_topology(). But MPIDR can not be trusted and is ignored for any topology deduction. And instead, topology of one single socket with multiple cores is made, which can not represent the real underlying system topology. I think this is the reason why VMs will in default prefer cores over sockets if no topology description is provided. With the feature introduced by this series, guest kernel can successfully get cpu information from one of the two (ACPI or DT) for topology population. According to above analysis, IMO, whether the parsing logic is "sockets over cores" or "cores over sockets", it just provide different topology information and consequently different scheduling performance. Apart from this, I think there will not any harm or problems caused. So maybe it's fine that we just use the arch-neutral parsing logic? How do you think ? Can you do an experiment where you create a guest with N vcpus, where N is the number of cores in a single socket. Then, pin each of those vcpus to a core in a single physical socket. Then, boot the VM with a topology of one socket and N cores and run some benchmarks. Then, boot the VM again with N sockets, one core each, and run the same benchmarks. I'm guessing we'll see the same benchmark numbers (within noise allowance) for both the runs. If we don't see the same numbers, then that'd be interesting. Yes, I can do the experiment, and will post the results later. Thanks, Yanan Thanks, drew .
Re: [RFC PATCH v2 2/4] hw/arm/virt: Parse -smp cluster parameter in virt_smp_parse
Hi Drew,
On 2021/4/28 18:31, Andrew Jones wrote:
On Tue, Apr 13, 2021 at 04:31:45PM +0800, Yanan Wang wrote:
There is a separate function virt_smp_parse() in hw/virt/arm.c used
to parse cpu topology for the ARM machines. So add parsing of -smp
cluster parameter in it, then total number of logical cpus will be
calculated like: max_cpus = sockets * clusters * cores * threads.
In virt_smp_parse(), the computing logic of missing values prefers
cores over sockets over threads. And for compatibility, the value
of clusters will be set as default 1 if not explicitly specified.
Signed-off-by: Yanan Wang
---
hw/arm/virt.c | 32 ++--
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 57ef961cb5..51797628db 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2639,35 +2639,38 @@ static void virt_smp_parse(MachineState *ms, QemuOpts
*opts)
if (opts) {
unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+unsigned clusters = qemu_opt_get_number(opts, "clusters", 1);
unsigned cores = qemu_opt_get_number(opts, "cores", 0);
unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+VirtMachineState *vms = VIRT_MACHINE(ms);
/*
- * Compute missing values; prefer cores over sockets and
- * sockets over threads.
+ * Compute missing values; prefer cores over sockets and sockets
+ * over threads. For compatibility, value of clusters will have
+ * been set as default 1 if not explicitly specified.
*/
if (cpus == 0 || cores == 0) {
sockets = sockets > 0 ? sockets : 1;
threads = threads > 0 ? threads : 1;
if (cpus == 0) {
cores = cores > 0 ? cores : 1;
-cpus = cores * threads * sockets;
+cpus = sockets * clusters * cores * threads;
} else {
ms->smp.max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus);
-cores = ms->smp.max_cpus / (sockets * threads);
+cores = ms->smp.max_cpus / (sockets * clusters * threads);
}
} else if (sockets == 0) {
threads = threads > 0 ? threads : 1;
-sockets = cpus / (cores * threads);
+sockets = cpus / (clusters * cores * threads);
sockets = sockets > 0 ? sockets : 1;
If we initialize clusters to zero instead of one and add lines in
'cpus == 0 || cores == 0' and 'sockets == 0' like
'clusters = clusters > 0 ? clusters : 1' as needed, then I think we can
add
} else if (clusters == 0) {
threads = threads > 0 ? threads : 1;
clusters = cpus / (sockets * cores * thread);
clusters = clusters > 0 ? clusters : 1;
}
here.
I have thought about this kind of format before, but there is a little bit
difference between these two ways. Let's chose the better and more
reasonable one of the two.
Way A currently in this patch:
If value of clusters is not explicitly specified in -smp command line,
we assume
that users don't want to support clusters, for compatibility we
initialized the
value to 1. So that with cmdline "-smp cpus=24, sockets=2, cores=6", we will
parse out the topology description like below:
cpus=24, sockets=2, clusters=1, cores=6, threads=2
Way B that you suggested for this patch:
Whether value of clusters is explicitly specified in -smp command line
or not,
we assume that clusters are supported and calculate the value. So that with
cmdline "-smp cpus=24, sockets=2, cores=6", we will parse out the topology
description like below:
cpus =24, sockets=2, clusters=2, cores=6, threads=1
But I think maybe we should not assume too much about what users think
through the -smp command line. We should just assume that all levels of
cpu topology are supported and calculate them, and users should be more
careful if they want to get the expected results with not so complete
cmdline.
If I'm right, then Way B should be better. :)
Thanks,
Yanan
} else if (threads == 0) {
-threads = cpus / (cores * sockets);
+threads = cpus / (sockets * clusters * cores);
threads = threads > 0 ? threads : 1;
-} else if (sockets * cores * threads < cpus) {
+} else if (sockets * clusters * cores * threads < cpus) {
error_report("cpu topology: "
- "sockets (%u) * cores (%u) * threads (%u) < "
- "smp_cpus (%u)",
- sockets, cores, threads, cpus);
+ "sockets (%u) * clusters (%u) * cores (%u) * "
+ "threads (%u) < smp_cpus (%u)",
+ sockets, clusters, cores, threads, cpus);
exit(1);
}
@@ -2678,11 +2681,11 @@ static void virt_smp_parse(MachineState *ms, QemuOpts
[PATCH v3] i386/cpu: Remove the deprecated cpu model 'Icelake-Client'
As it's been marked deprecated since v5.2, now I think it's time remove it
from code.
Signed-off-by: Robert Hoo
---
Changelog:
v3:
Update deprecated.rst. (Sorry for my carelessness in last search. I
sware I did search.)
v2:
Update removed-features.rst.
---
docs/system/deprecated.rst | 6 --
docs/system/removed-features.rst | 5 ++
target/i386/cpu.c| 118 ---
3 files changed, 5 insertions(+), 124 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 80cae86..780b756 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -222,12 +222,6 @@ a future version of QEMU. Support for this CPU was removed
from the
upstream Linux kernel, and there is no available upstream toolchain
to build binaries for it.
-``Icelake-Client`` CPU Model (since 5.2.0)
-''
-
-``Icelake-Client`` CPU Models are deprecated. Use ``Icelake-Server`` CPU
-Models instead.
-
MIPS ``I7200`` CPU Model (since 5.2)
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index 29e9060..f1b5a16 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -285,6 +285,11 @@ The RISC-V no MMU cpus have been removed. The two CPUs:
``rv32imacu-nommu`` and
``rv64imacu-nommu`` can no longer be used. Instead the MMU status can be
specified
via the CPU ``mmu`` option when using the ``rv32`` or ``rv64`` CPUs.
+x86 Icelake-Client CPU (removed in 6.1)
+'''
+
+``Icelake-Client`` cpu can no longer be used. Use ``Icelake-Server`` instead.
+
System emulator machines
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ad99cad..75f2ad1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3338,124 +3338,6 @@ static X86CPUDefinition builtin_x86_defs[] = {
.model_id = "Intel Xeon Processor (Cooperlake)",
},
{
-.name = "Icelake-Client",
-.level = 0xd,
-.vendor = CPUID_VENDOR_INTEL,
-.family = 6,
-.model = 126,
-.stepping = 0,
-.features[FEAT_1_EDX] =
-CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
-CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA |
-CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
-CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
-CPUID_DE | CPUID_FP87,
-.features[FEAT_1_ECX] =
-CPUID_EXT_AVX | CPUID_EXT_XSAVE | CPUID_EXT_AES |
-CPUID_EXT_POPCNT | CPUID_EXT_X2APIC | CPUID_EXT_SSE42 |
-CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_SSSE3 |
-CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3 |
-CPUID_EXT_TSC_DEADLINE_TIMER | CPUID_EXT_FMA | CPUID_EXT_MOVBE |
-CPUID_EXT_PCID | CPUID_EXT_F16C | CPUID_EXT_RDRAND,
-.features[FEAT_8000_0001_EDX] =
-CPUID_EXT2_LM | CPUID_EXT2_RDTSCP | CPUID_EXT2_NX |
-CPUID_EXT2_SYSCALL,
-.features[FEAT_8000_0001_ECX] =
-CPUID_EXT3_ABM | CPUID_EXT3_LAHF_LM | CPUID_EXT3_3DNOWPREFETCH,
-.features[FEAT_8000_0008_EBX] =
-CPUID_8000_0008_EBX_WBNOINVD,
-.features[FEAT_7_0_EBX] =
-CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 |
-CPUID_7_0_EBX_HLE | CPUID_7_0_EBX_AVX2 | CPUID_7_0_EBX_SMEP |
-CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ERMS | CPUID_7_0_EBX_INVPCID |
-CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_RDSEED | CPUID_7_0_EBX_ADX |
-CPUID_7_0_EBX_SMAP,
-.features[FEAT_7_0_ECX] =
-CPUID_7_0_ECX_AVX512_VBMI | CPUID_7_0_ECX_UMIP | CPUID_7_0_ECX_PKU
|
-CPUID_7_0_ECX_AVX512_VBMI2 | CPUID_7_0_ECX_GFNI |
-CPUID_7_0_ECX_VAES | CPUID_7_0_ECX_VPCLMULQDQ |
-CPUID_7_0_ECX_AVX512VNNI | CPUID_7_0_ECX_AVX512BITALG |
-CPUID_7_0_ECX_AVX512_VPOPCNTDQ,
-.features[FEAT_7_0_EDX] =
-CPUID_7_0_EDX_SPEC_CTRL | CPUID_7_0_EDX_SPEC_CTRL_SSBD,
-/* Missing: XSAVES (not supported by some Linux versions,
-* including v4.1 to v4.12).
-* KVM doesn't yet expose any XSAVES state save component,
-* and the only one defined in Skylake (processor tracing)
-* probably will block migration anyway.
-*/
-.features[FEAT_XSAVE] =
-CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
-CPUID_XSAVE_XGETBV1,
-.features[FEAT_6_EAX] =
-CPUID_6_EAX_ARAT,
-/* Missing: Mode-based execute control (XS/XU), processor tracing, TSC
scaling */
-.features[FEAT_VMX_BASIC] = MSR_VMX_BASIC_INS_OUTS |
- MSR_VMX_BASIC_TRUE_CTLS,
-.features[FEAT_VMX_ENTRY_CTLS] = VMX_VM_ENTRY_IA32E_MODE |
- VMX_VM_ENTRY_LOAD_IA32_PERF_GLO
Re: [RFC PATCH v2 1/4] vl.c: Add -smp, clusters=* command line support for ARM cpu
On 2021/4/28 18:23, Andrew Jones wrote:
On Tue, Apr 13, 2021 at 04:31:44PM +0800, Yanan Wang wrote:
A cluster means a group of cores that share some resources (e.g. cache)
among them under the LLC. For example, ARM64 server chip Kunpeng 920 has
6 or 8 clusters in each NUMA, and each cluster has 4 cores. All clusters
share L3 cache data while cores within each cluster share the L2 cache.
The cache affinity of cluster has been proved to improve the Linux kernel
scheduling performance and a patchset has been posted, in which a general
sched_domain for clusters was added and a cluster level was added in the
arch-neutral cpu topology struct like below.
struct cpu_topology {
int thread_id;
int core_id;
int cluster_id;
int package_id;
int llc_id;
cpumask_t thread_sibling;
cpumask_t core_sibling;
cpumask_t cluster_sibling;
cpumask_t llc_sibling;
}
Also the Kernel Doc: Documentation/devicetree/bindings/cpu/cpu-topology.txt
defines a four-level CPU topology hierarchy like socket/cluster/core/thread.
According to the context, a socket node's child nodes must be one or more
cluster nodes and a cluster node's child nodes must be one or more cluster
nodes/one or more core nodes.
So let's add the -smp, clusters=* command line support for ARM cpu, so that
future guest os could make use of cluster cpu topology for better scheduling
performance. For ARM machines, a four-level cpu hierarchy can be defined and
it will be sockets/clusters/cores/threads. Because we only support clusters
for ARM cpu currently, a new member "unsigned smp_clusters" is added to the
VirtMachineState structure.
Signed-off-by: Yanan Wang
---
include/hw/arm/virt.h | 1 +
qemu-options.hx | 26 +++---
softmmu/vl.c | 3 +++
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 4a4b98e4a7..5d5d156924 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -155,6 +155,7 @@ struct VirtMachineState {
char *pciehb_nodename;
const int *irqmap;
int fdt_size;
+unsigned smp_clusters;
uint32_t clock_phandle;
uint32_t gic_phandle;
uint32_t msi_phandle;
diff --git a/qemu-options.hx b/qemu-options.hx
index fd21002bd6..65343ea23c 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -184,25 +184,29 @@ SRST
ERST
DEF("smp", HAS_ARG, QEMU_OPTION_smp,
-"-smp
[cpus=]n[,maxcpus=cpus][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
+"-smp
[cpus=]n[,maxcpus=cpus][,clusters=clusters][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]\n"
Please put clusters directly in front of dies in the above list, like it
is in the list description below
"set the number of CPUs to 'n' [default=1]\n"
"maxcpus= maximum number of total cpus, including\n"
"offline CPUs for hotplug, etc\n"
-"cores= number of CPU cores on one socket (for PC, it's on one
die)\n"
+"cores= number of CPU cores on one socket\n"
+"(it's on one die for PC, and on one cluster for ARM)\n"
"threads= number of threads on one CPU core\n"
+"clusters= number of CPU clusters on one socket (for ARM
only)\n"
"dies= number of CPU dies on one socket (for PC only)\n"
"sockets= number of discrete sockets in the system\n",
QEMU_ARCH_ALL)
SRST
-``-smp
[cpus=]n[,cores=cores][,threads=threads][,dies=dies][,sockets=sockets][,maxcpus=maxcpus]``
-Simulate an SMP system with n CPUs. On the PC target, up to 255 CPUs
-are supported. On Sparc32 target, Linux limits the number of usable
-CPUs to 4. For the PC target, the number of cores per die, the
-number of threads per cores, the number of dies per packages and the
-total number of sockets can be specified. Missing values will be
-computed. If any on the three values is given, the total number of
-CPUs n can be omitted. maxcpus specifies the maximum number of
-hotpluggable CPUs.
+``-smp
[cpus=]n[,maxcpus=cpus][,clusters=clusters][,cores=cores][,threads=threads][,dies=dies][,sockets=sockets]``
Also move clusters in this list over in front of dies to match the
suggested change above.
Thanks, I will change the place.
Thanks,
Yanan
+Simulate an SMP system with n CPUs. On the PC target, up to 255
+CPUs are supported. On the Sparc32 target, Linux limits the number
+of usable CPUs to 4. For the PC target, the number of threads per
+core, the number of cores per die, the number of dies per package
+and the total number of sockets can be specified. For the ARM target,
+the number of threads per core, the number of cores per cluster, the
+number of clusters per socket and the total number of sockets can be
+specified. And missing values will be computed. If a
Re: [PATCH RESEND v2] i386/cpu: Remove the deprecated cpu model 'Icelake-Client'
On Wed, 2021-04-28 at 11:24 -0400, Eduardo Habkost wrote:
> On Wed, Apr 28, 2021 at 10:41:13AM +0800, Robert Hoo wrote:
> > As it's been marked deprecated since v5.2, now I think it's time
> > remove it
> > from code.
> >
> > Signed-off-by: Robert Hoo
> > ---
> > (Sorry, forgot to append changelog in last send.)
> > Changelog:
> > v2:
> > Update removed-features.rst.
> > Since previously no its deprecation info was recorded in
> > docs/system/deprecated.rst, nothing to update in it.
>
> deprecated.rst has an entry for Icelake-Client. See commit
> 3e6a015cbd0f ("i386: Mark Icelake-Client CPU models deprecated").
>
Oh, my carelessness in the search. Thanks Eduardo!
Going to send v3 soon.
Re: X on old (non-x86) Linux guests
On Wed, 28 Apr 2021, Dr. David Alan Gilbert wrote: * BALATON Zoltan (bala...@eik.bme.hu) wrote: On Wed, 28 Apr 2021, Andrew Randrianasulu wrote: On Wednesday, April 28, 2021, Andrew Randrianasulu wrote: On Monday, April 26, 2021, BALATON Zoltan wrote: On Mon, 26 Apr 2021, Dr. David Alan Gilbert wrote: Over the weekend I got a Red Hat 6.x (not RHEL!) for Alpha booting under QEMU which was pretty neat. But I failed to find a succesful combination to get X working; has anyone any suggestions? Adding Andrew who has experimented with old X framebuffer so he may remember something more but that was on x86. Sorry, I still away from my desktop (with notes/logs), not sure when return.. I do not think I tried something that old.. Kernel 2.2 i guess, before any attempt at r128 drm Kernel module was written (in 2.4?) and so before ddx attempted to use that (as it tries by default in much newer distros) Maybe it would work better with newer RedHat than 6.0? I think I've seen images up to at least 7.1 that supported alpha but I don't know how to boot them. I could get kernel and installer running with -kernel -initrd but did not find the CD on the defailt CMD646 controller (seems to only have driver for one SCSI controller) so I'm not sure how to try this. Trying to just boot from the CD without -kernel -initrd it just stops after displaying "Hello" in top left but that could be something about alpha firmware I don't know how to use. I ended up using -kernel/-initrd and passed the cdrom as an image to hdb rather than with -cdrom; as you say the cmd646 didn't like the cdrom. (Where I'm pretty sure my real Alpha does have a CDROM connected to it's cmd646). And none of the SCSI controllers would work. Passing the iso as HD did not work with 7.1 iso as that does not seem to be hybrid or the kernel said no partition found. I'll make some notes on my command line this weekend and post them next week; I'll try the X suggestions as well. It looks like RedHat versions before 7.1 don't have any fb drivers (not sure those existed on kernel 2.2), the 7.1 iso has kernel 2.4 but only seems to have radeonfb and r128 drm driver which may not work as it probably wants to use 3D or other features we don't emulate yet. If you can't compile a kernel with aty128fb you may try the provided radeonfb with -device ati-vga,model=rv100 but not sure if that will load or work. If you can get picture with that then X using fb driver may work. Not sure if the X r128 driver would work with -device ati-vga but I'm quite sure a radeon driver will not work yet even if you set model to rv100 as those drivers usually want to use memory queues that we don't emulate yet. It also seems there was once a 7.2 iso available from Compaq but could not find a place that still has it so don't know if that would be any better. Maybe you can try other distros too to see if those have more fb drivers. Regards, BALATON Zoltan
Re: [PATCH v4 00/26] Hexagon (target/hexagon) update
On 4/28/21 4:20 PM, Taylor Simpson wrote: I get -Wno-unused-function added to the compiler command line, so I don't see the error. Ah, looks like it's the version of glib on your system. The flag gets added in configure beneath: # Silence clang warnings triggered by glib < 2.57.2 Both were introduced in patch 22/26. Should I fix this by respinning the series or sending a single patch? I can cherry-pick a single patch. r~
RE: [PATCH v4 00/26] Hexagon (target/hexagon) update
> -Original Message-
> From: Richard Henderson
> Sent: Wednesday, April 28, 2021 4:13 PM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: phi...@redhat.com; a...@rev.ng; Brian Cain
> Subject: Re: [PATCH v4 00/26] Hexagon (target/hexagon) update
>
> On 4/8/21 6:07 PM, Taylor Simpson wrote:
> > This patch series is a significant update for the Hexagon target
> > The first 16 patches address feedback from Richard Henderson
> >and Philippe Mathieu-
> Daud�
> > The next 10 patches add the remaining instructions for the Hexagon
> > scalar core
> >
> > The patches are logically independent but are organized as a series to
> > avoid potential conflicts if they are merged out of order.
> >
> > Note that the new test cases require an updated toolchain/container.
>
> https://gitlab.com/rth7680/qemu/-/jobs/1216248227
>
> The clang-user job errors out with
>
>
> > ../target/hexagon/genptr.c:31:20: error: unused function 'gen_read_reg' [-
> Werror,-Wunused-function]
> > static inline TCGv gen_read_reg(TCGv result, int num)
> >^
> > ../target/hexagon/genptr.c:322:20: error: unused function 'gen_set_byte'
> [-Werror,-Wunused-function]
> > static inline void gen_set_byte(int N, TCGv result, TCGv src)
> >^
My apologies!
What's the value of $CONFIG_ARGS that is referenced here
$ if test -n "$TARGETS"; then ../configure --enable-werror --disable-docs
$CONFIGURE_ARGS --target-list="$TARGETS" ; else ../configure --enable-werror
--disable-docs $CONFIGURE_ARGS ; fi || { cat config.log
meson-logs/meson-log.txt && exit 1; }
When I configure with
../configure --enable-werror --cc=clang --disable-docs
--target-list=hexagon-linux-user
I get -Wno-unused-function added to the compiler command line, so I don't see
the error.
Both were introduced in patch 22/26. Should I fix this by respinning the
series or sending a single patch?
Taylor
[Bug 1926521] [NEW] QEMU-user ignores MADV_DONTNEED
Public bug reported:
There is comment int the code "This is a hint, so ignoring and returning
success is ok"
https://github.com/qemu/qemu/blob/b1cffefa1b163bce9aebc3416f562c1d3886eeaa/linux-user/syscall.c#L11941
But it seems incorrect with the current state of Linux
"man madvise" or https://man7.org/linux/man-pages/man2/madvise.2.html
says the following:
>> These advice values do not influence the semantics
>> of the application (except in the case of MADV_DONTNEED)
>> After a successful MADV_DONTNEED operation, the semantics
>> of memory access in the specified region are changed:
>> subsequent accesses of pages in the range will succeed,
>> but will result in either repopulating the memory contents
>> from the up-to-date contents of the underlying mapped file
>> (for shared file mappings, shared anonymous mappings, and
>> shmem-based techniques such as System V shared memory
>> segments) or zero-fill-on-demand pages for anonymous
>> private mappings.
Some applications use this behavior clear memory and it
would be nice to be able to run them on QEMU without
workarounds.
Reproducer on "Debian 5.10.24 x86_64 GNU/Linux" as a host.
```
#include "assert.h"
#include "stdio.h"
#include
#include
int main() {
char *P = (char *)mmap(0, 4096, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
assert(P);
*P = 'A';
while (madvise(P, 4096, MADV_DONTNEED) == -1 && errno == EAGAIN) {
}
assert(*P == 0);
printf("OK\n");
}
/*
gcc /tmp/madvice.c -o /tmp/madvice
qemu-x86_64 /tmp/madvice
madvice: /tmp/madvice.c:13: main: Assertion `*P == 0' failed.
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted
/tmp/madvice
OK
*/
```
** Affects: qemu
Importance: Undecided
Status: New
** Summary changed:
- QEMU-user does no respect MADV_DONTNEED
+ QEMU-user ignores MADV_DONTNEED
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1926521
Title:
QEMU-user ignores MADV_DONTNEED
Status in QEMU:
New
Bug description:
There is comment int the code "This is a hint, so ignoring and returning
success is ok"
https://github.com/qemu/qemu/blob/b1cffefa1b163bce9aebc3416f562c1d3886eeaa/linux-user/syscall.c#L11941
But it seems incorrect with the current state of Linux
"man madvise" or https://man7.org/linux/man-pages/man2/madvise.2.html
says the following:
>> These advice values do not influence the semantics
>> of the application (except in the case of MADV_DONTNEED)
>> After a successful MADV_DONTNEED operation, the semantics
>> of memory access in the specified region are changed:
>> subsequent accesses of pages in the range will succeed,
>> but will result in either repopulating the memory contents
>> from the up-to-date contents of the underlying mapped file
>> (for shared file mappings, shared anonymous mappings, and
>> shmem-based techniques such as System V shared memory
>> segments) or zero-fill-on-demand pages for anonymous
>> private mappings.
Some applications use this behavior clear memory and it
would be nice to be able to run them on QEMU without
workarounds.
Reproducer on "Debian 5.10.24 x86_64 GNU/Linux" as a host.
```
#include "assert.h"
#include "stdio.h"
#include
#include
int main() {
char *P = (char *)mmap(0, 4096, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
assert(P);
*P = 'A';
while (madvise(P, 4096, MADV_DONTNEED) == -1 && errno == EAGAIN) {
}
assert(*P == 0);
printf("OK\n");
}
/*
gcc /tmp/madvice.c -o /tmp/madvice
qemu-x86_64 /tmp/madvice
madvice: /tmp/madvice.c:13: main: Assertion `*P == 0' failed.
qemu: uncaught target signal 6 (Aborted) - core dumped
Aborted
/tmp/madvice
OK
*/
```
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1926521/+subscriptions
RE: [RUST] Add crate for generic vhost-user-i2c backend daemon
Hi, I didn't realize that you had already CCed the rust-vmm mailing list, so anyways it is a good start. -Original Message- From: Trilok Soni Sent: Wednesday, April 28, 2021 10:14 AM To: Viresh Kumar ; stratos-...@op-lists.linaro.org; rust-...@lists.opendev.org Cc: Vincent Guittot ; Mike Holmes ; Bill Mills ; Alex Bennée ; Arnd Bergmann ; Jie Deng ; qemu-devel@nongnu.org Subject: RE: [RUST] Add crate for generic vhost-user-i2c backend daemon Viresh, For rust-vmm, you need to create the new issue in the right project. You can probably pick up vmm-reference project at rust-vmm and ask for the new crate. You can also send email to rust-vmm mailing list but github "issues" feature is used heavily in the rust-vmm project. There is also bi-weekly meetings which is attended by me, Vatsa and rust-vmm developers where it can be put up as agenda. The minimal requirement for the new crate is to have less (or almost none) dependencies on other crates so that they can be independently tested in the rust-vmm CI. Anyways, please file a new issue and I will ask Vatsa and others to comment there. https://github.com/rust-vmm/vhost-user-backend http://lists.opendev.org/pipermail/rust-vmm/2021-March/000406.html ---Trilok Soni -Original Message- From: Viresh Kumar Sent: Wednesday, April 28, 2021 5:23 AM To: stratos-...@op-lists.linaro.org; rust-...@lists.opendev.org Cc: Vincent Guittot ; Mike Holmes ; Bill Mills ; Alex Bennée ; Arnd Bergmann ; Jie Deng ; qemu-devel@nongnu.org; Trilok Soni Subject: [RUST] Add crate for generic vhost-user-i2c backend daemon - CAUTION: This email originated from outside of the organization. - Hello, In my earlier attempt [1], I implemented the vhost-user-i2c backend deamon for QEMU (though the code was generic enough to be used with any hypervisor). And here is a Rust implementation of the vhost-user-i2c backend daemon. Again this is generic enough to be used with any hypervisor and can live in its own repository now: https://github.com/vireshk/vhost-user-i2c I am not sure what's the process to get this merged to Rust-vmm. Can someone help ? Is that the right thing to do ? -8<- Here are other details (which are same since the earlier Qemu attempt): This is an initial implementation of a generic vhost-user backend for the I2C bus. This is based of the virtio specifications (already merged) for the I2C bus. The kernel virtio I2C driver is still under review, here [2] is the latest version (v10): The backend is implemented as a vhost-user device because we want to experiment in making portable backends that can be used with multiple hypervisors. We also want to support backends isolated in their own separate service VMs with limited memory cross-sections with the principle guest. This is part of a wider initiative by Linaro called "project Stratos" for which you can find information here: https://collaborate.linaro.org/display/STR/Stratos+Home I2C Testing: I didn't have access to a real hardware where I can play with a I2C client device (like RTC, eeprom, etc) to verify the working of the backend daemon, so I decided to test it on my x86 box itself with hierarchy of two ARM64 guests. The first ARM64 guest was passed "-device ds1338,address=0x20" option, so it could emulate a ds1338 RTC device, which connects to an I2C bus. Once the guest came up, ds1338 device instance was created within the guest kernel by doing: echo ds1338 0x20 > /sys/bus/i2c/devices/i2c-0/new_device [ Note that this may end up binding the ds1338 device to its driver, which won't let our i2c daemon talk to the device. For that we need to manually unbind the device from the driver: echo 0-0020 > /sys/bus/i2c/devices/0-0020/driver/unbind ] After this is done, you will get /dev/rtc1. This is the device we wanted to emulate, which will be accessed by the vhost-user-i2c backend daemon via the /dev/i2c-0 file present in the guest VM. At this point we need to start the backend daemon and give it a socket-path to talk to from qemu (you can pass -v to it to get more detailed messages): target/debug/vhost-user-i2c --socket-path=vi2c.sock -l 0:32 [ Here, 0:32 is the bus/device mapping, 0 for /dev/i2c-0 and 32 (i.e. 0x20) is client address of ds1338 that we used while creating the device. ] Now we need to start the second level ARM64 guest (from within the first guest) to get the i2c-virtio.c Linux driver up. The second level guest is passed the following options to connect to the same socket: -chardev socket,path=vi2c.sock,id=vi2c \ -device vhost-user-i2c-pci,chardev=vi2c,id=i2c Once the second level guest boots up, we will see the i2c-virtio bus at /sys/bus/i2c/devices/i2c-X/. From there we can now make it emulate the ds1338 devi
RE: [RUST] Add crate for generic vhost-user-i2c backend daemon
Viresh, For rust-vmm, you need to create the new issue in the right project. You can probably pick up vmm-reference project at rust-vmm and ask for the new crate. You can also send email to rust-vmm mailing list but github "issues" feature is used heavily in the rust-vmm project. There is also bi-weekly meetings which is attended by me, Vatsa and rust-vmm developers where it can be put up as agenda. The minimal requirement for the new crate is to have less (or almost none) dependencies on other crates so that they can be independently tested in the rust-vmm CI. Anyways, please file a new issue and I will ask Vatsa and others to comment there. https://github.com/rust-vmm/vhost-user-backend http://lists.opendev.org/pipermail/rust-vmm/2021-March/000406.html ---Trilok Soni -Original Message- From: Viresh Kumar Sent: Wednesday, April 28, 2021 5:23 AM To: stratos-...@op-lists.linaro.org; rust-...@lists.opendev.org Cc: Vincent Guittot ; Mike Holmes ; Bill Mills ; Alex Bennée ; Arnd Bergmann ; Jie Deng ; qemu-devel@nongnu.org; Trilok Soni Subject: [RUST] Add crate for generic vhost-user-i2c backend daemon - CAUTION: This email originated from outside of the organization. - Hello, In my earlier attempt [1], I implemented the vhost-user-i2c backend deamon for QEMU (though the code was generic enough to be used with any hypervisor). And here is a Rust implementation of the vhost-user-i2c backend daemon. Again this is generic enough to be used with any hypervisor and can live in its own repository now: https://github.com/vireshk/vhost-user-i2c I am not sure what's the process to get this merged to Rust-vmm. Can someone help ? Is that the right thing to do ? -8<- Here are other details (which are same since the earlier Qemu attempt): This is an initial implementation of a generic vhost-user backend for the I2C bus. This is based of the virtio specifications (already merged) for the I2C bus. The kernel virtio I2C driver is still under review, here [2] is the latest version (v10): The backend is implemented as a vhost-user device because we want to experiment in making portable backends that can be used with multiple hypervisors. We also want to support backends isolated in their own separate service VMs with limited memory cross-sections with the principle guest. This is part of a wider initiative by Linaro called "project Stratos" for which you can find information here: https://collaborate.linaro.org/display/STR/Stratos+Home I2C Testing: I didn't have access to a real hardware where I can play with a I2C client device (like RTC, eeprom, etc) to verify the working of the backend daemon, so I decided to test it on my x86 box itself with hierarchy of two ARM64 guests. The first ARM64 guest was passed "-device ds1338,address=0x20" option, so it could emulate a ds1338 RTC device, which connects to an I2C bus. Once the guest came up, ds1338 device instance was created within the guest kernel by doing: echo ds1338 0x20 > /sys/bus/i2c/devices/i2c-0/new_device [ Note that this may end up binding the ds1338 device to its driver, which won't let our i2c daemon talk to the device. For that we need to manually unbind the device from the driver: echo 0-0020 > /sys/bus/i2c/devices/0-0020/driver/unbind ] After this is done, you will get /dev/rtc1. This is the device we wanted to emulate, which will be accessed by the vhost-user-i2c backend daemon via the /dev/i2c-0 file present in the guest VM. At this point we need to start the backend daemon and give it a socket-path to talk to from qemu (you can pass -v to it to get more detailed messages): target/debug/vhost-user-i2c --socket-path=vi2c.sock -l 0:32 [ Here, 0:32 is the bus/device mapping, 0 for /dev/i2c-0 and 32 (i.e. 0x20) is client address of ds1338 that we used while creating the device. ] Now we need to start the second level ARM64 guest (from within the first guest) to get the i2c-virtio.c Linux driver up. The second level guest is passed the following options to connect to the same socket: -chardev socket,path=vi2c.sock,id=vi2c \ -device vhost-user-i2c-pci,chardev=vi2c,id=i2c Once the second level guest boots up, we will see the i2c-virtio bus at /sys/bus/i2c/devices/i2c-X/. From there we can now make it emulate the ds1338 device again by doing: echo ds1338 0x20 > /sys/bus/i2c/devices/i2c-0/new_device [ This time we want ds1338's driver to be bound to the device, so it should be enabled in the kernel as well. ] And we will get /dev/rtc1 device again here in the second level guest. Now we can play with the rtc device with help of hwclock utility and we can see the following sequence of transfers happening if we try to update rtc's time from system time. hwclock -w -f /dev/r
[Bug 1926497] Re: dp83932 stops working after a short while
It looks like using https://cdimage.debian.org/cdimage/ports/snapshots/2021-04-17/debian-10.0.0 -m68k-NETINST-1.iso instead fixes the issue. Perhaps the instruction on https://wiki.qemu.org/Documentation/Platforms/m68k should be updated. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1926497 Title: dp83932 stops working after a short while Status in QEMU: New Bug description: Following the instructions here https://wiki.qemu.org/Documentation/Platforms/m68k I was able to successfully install debian. However, running apt-get update stalls after the first 1-2MB. root@debian:~# apt-get update Get:1 http://ftp.ports.debian.org/debian-ports sid InRelease [55.3 kB] Ign:1 http://ftp.ports.debian.org/debian-ports sid InRelease Get:2 http://ftp.ports.debian.org/debian-ports sid/main all Packages [8,735 kB] 18% [2 Packages 2,155 kB/8,735 kB 25%] After running apt-get update. I don't seem to be able to send any packets anymore. ping host lookups fail and a subsequent apt-get update makes no progress. I'm launching qemu with: qemu-system-m68k -boot c \ -M q800 -serial none -serial mon:stdio -m 1000M \ -net nic,model=dp83932 -net user \ -append "root=/dev/sda2 rw console=ttyS0 console=tty" \ -kernel vmlinux-4.16.0-1-m68k \ -initrd initrd.img-4.16.0-1-m68k \ -drive file=m68k-deb10.qcow2,format=qcow2 \ -nographic I see this with qemu v6.0.0-rc5 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1926497/+subscriptions
Re: [PATCH v2] target/i386: add "-cpu, lbr-fmt=*" support to enable guest LBR
On Tue, Apr 27, 2021 at 04:09:48PM +0800, Like Xu wrote:
> The last branch recording (LBR) is a performance monitor unit (PMU)
> feature on Intel processors that records a running trace of the most
> recent branches taken by the processor in the LBR stack. The QEMU
> could configure whether it's enabled or not for each guest via CLI.
>
> The LBR feature would be enabled on the guest if:
> - the KVM is enabled and the PMU is enabled and,
> - the msr-based-feature IA32_PERF_CAPABILITIES is supporterd on KVM and,
> - the supported returned value for lbr_fmt from this msr is not zero and,
> - the requested guest vcpu model does support FEAT_1_ECX.CPUID_EXT_PDCM,
> - the configured lbr-fmt value is the same as the host lbr_fmt value
> OR use the QEMU option "-cpu host,migratable=no".
I don't understand why "migratable" matters here. "migratable"
is just a convenience property to get better defaults when using
"-cpu host". I don't know why it would change the lbr-fmt
validation rules.
>
> Signed-off-by: Like Xu
> ---
A changelog explaining what you changed since v1 would have been
useful here.
> target/i386/cpu.c | 34 ++
> target/i386/cpu.h | 10 ++
> target/i386/kvm/kvm.c | 10 --
> 3 files changed, 52 insertions(+), 2 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ad99cad0e7..9c8e54aa6f 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6623,6 +6623,10 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool
> verbose)
> }
>
> for (w = 0; w < FEATURE_WORDS; w++) {
> +if (w == FEAT_PERF_CAPABILITIES) {
> +continue;
> +}
> +
Why exactly is this necessary? I expected to be completely OK to
call mark_unavailable_features() multiple times for the same
FeatureWord.
If there's a reason why this is necessary, I suggest adding a
comment explaining why.
> uint64_t host_feat =
> x86_cpu_get_supported_feature_word(w, false);
> uint64_t requested_features = env->features[w];
> @@ -6630,6 +6634,27 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool
> verbose)
> mark_unavailable_features(cpu, w, unavailable_features, prefix);
> }
>
> +uint64_t host_perf_cap =
> +x86_cpu_get_supported_feature_word(FEAT_PERF_CAPABILITIES, false);
> +if (!cpu->lbr_fmt && !cpu->migratable) {
> +cpu->lbr_fmt = host_perf_cap & PERF_CAP_LBR_FMT;
"migratable=no" is not a request to override explicit user
settings. This is why we have the ~env->user_features masking
inside x86_cpu_expand_features() when initializing
env->features[].
In either case, I don't understand why you need the lines above.
"migratable=no" should already trigger the x86_cpu_get_supported_feature_word()
loop inside x86_cpu_expand_features(), and it should initialize
env->features[FEAT_PERF_CAPABILITIES] with the host value. Isn't
that code working for you?
> +if (cpu->lbr_fmt) {
> +info_report("vPMU: The value of lbr-fmt has been adjusted "
> +"to 0x%lx and guest LBR is enabled.",
> +host_perf_cap & PERF_CAP_LBR_FMT);
>From your other message:
(I'm assuming your examples are for a lbr-fmt=5 host)
> "-cpu host,migratable=no" --> "Enable guest LBR and show warning"
Enabling guest LBR in this case is 100% OK, isn't it? I don't
think you need to show a warning.
> "-cpu host,migratable=no,lbr-fmt=0" --> "Enable guest LBR and show warning"
Why? In this case, we should do what the user asked for whenever
possible, and the user is explicitly asking lbr-fmt to be 0.
> "-cpu host,migratable=no,lbr-fmt=5" --> "Enable guest LBR"
Looks OK.
> "-cpu host,migratable=no,lbr-fmt=6" --> "Disable guest LBR and show warning"
Makes sense to me[1].
> +}
> +} else {
> +uint64_t requested_lbr_fmt = cpu->lbr_fmt & PERF_CAP_LBR_FMT;
> +if (requested_lbr_fmt && kvm_enabled()) {
>From your other message:
> "-cpu host,lbr-fmt=0" --> "Disable guest LBR"
Makes sense to me. I understand this as a confirmation that it's
OK to have a guest/host mismatch if guest LBR=0.
> "-cpu host,lbr-fmt=5" --> "Enable guest LBR"
Makes sense to me.
> "-cpu host,lbr-fmt=6" --> "Disable guest LBR and show warning"
Makes sense to me[1].
[1] As long as "show warning" becomes "fatal error" if enforce=1.
mark_unavailable_features() should make sure this happens.
Or maybe we should make this an error? It would be even
better. The example code below makes it an error.
> +if (requested_lbr_fmt != (host_perf_cap & PERF_CAP_LBR_FMT)) {
> +cpu->lbr_fmt = 0;
> +warn_report("vPMU: The supported lbr-fmt value on the host "
> +"is 0x%lx and guest LBR is disabled.",
> +host_perf_cap & PERF_CAP_LBR_FMT);
> +}
> +}
> +}
> +
> if ((env->features[F
Re: [PATCH v4 00/26] Hexagon (target/hexagon) update
On 4/8/21 6:07 PM, Taylor Simpson wrote: This patch series is a significant update for the Hexagon target The first 16 patches address feedback from Richard Henderson and Philippe Mathieu-Daud� The next 10 patches add the remaining instructions for the Hexagon scalar core The patches are logically independent but are organized as a series to avoid potential conflicts if they are merged out of order. Note that the new test cases require an updated toolchain/container. https://gitlab.com/rth7680/qemu/-/jobs/1216248227 The clang-user job errors out with ../target/hexagon/genptr.c:31:20: error: unused function 'gen_read_reg' [-Werror,-Wunused-function] static inline TCGv gen_read_reg(TCGv result, int num) ^ ../target/hexagon/genptr.c:322:20: error: unused function 'gen_set_byte' [-Werror,-Wunused-function] static inline void gen_set_byte(int N, TCGv result, TCGv src) ^ r~
[Bug 1926497] Re: dp83932 stops working after a short while
The kernel in my m68k disk image is vmlinux-4.16.0-1-m68k which is presumably what comes from https://cdimage.debian.org/cdimage/ports/10.0/m68k/iso-cd/debian-10.0 -m68k-NETINST-1.iso. Is there a debian image that uses a newer kernel? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1926497 Title: dp83932 stops working after a short while Status in QEMU: New Bug description: Following the instructions here https://wiki.qemu.org/Documentation/Platforms/m68k I was able to successfully install debian. However, running apt-get update stalls after the first 1-2MB. root@debian:~# apt-get update Get:1 http://ftp.ports.debian.org/debian-ports sid InRelease [55.3 kB] Ign:1 http://ftp.ports.debian.org/debian-ports sid InRelease Get:2 http://ftp.ports.debian.org/debian-ports sid/main all Packages [8,735 kB] 18% [2 Packages 2,155 kB/8,735 kB 25%] After running apt-get update. I don't seem to be able to send any packets anymore. ping host lookups fail and a subsequent apt-get update makes no progress. I'm launching qemu with: qemu-system-m68k -boot c \ -M q800 -serial none -serial mon:stdio -m 1000M \ -net nic,model=dp83932 -net user \ -append "root=/dev/sda2 rw console=ttyS0 console=tty" \ -kernel vmlinux-4.16.0-1-m68k \ -initrd initrd.img-4.16.0-1-m68k \ -drive file=m68k-deb10.qcow2,format=qcow2 \ -nographic I see this with qemu v6.0.0-rc5 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1926497/+subscriptions
Re: [PATCH 5/5] vhost-user-blk: Check that num-queues is supported by backend
Reviewed-by: Raphael Norwitz
On Thu, Apr 22, 2021 at 07:02:21PM +0200, Kevin Wolf wrote:
> Creating a device with a number of queues that isn't supported by the
> backend is pointless, the device won't work properly and the error
> messages are rather confusing.
>
> Just fail to create the device if num-queues is higher than what the
> backend supports.
>
> Since the relationship between num-queues and the number of virtqueues
> depends on the specific device, this is an additional value that needs
> to be initialised by the device. For convenience, allow leaving it 0 if
> the check should be skipped. This makes sense for vhost-user-net where
> separate vhost devices are used for the queues and custom initialisation
> code is needed to perform the check.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935031
> Signed-off-by: Kevin Wolf
> ---
> include/hw/virtio/vhost.h | 2 ++
> hw/block/vhost-user-blk.c | 1 +
> hw/virtio/vhost-user.c| 5 +
> 3 files changed, 8 insertions(+)
>
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 4a8bc75415..21a9a52088 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -74,6 +74,8 @@ struct vhost_dev {
> int nvqs;
> /* the first virtqueue which would be used by this vhost dev */
> int vq_index;
> +/* if non-zero, minimum required value for max_queues */
> +int num_queues;
> uint64_t features;
> uint64_t acked_features;
> uint64_t backend_features;
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index b6f4bb3f6f..ac2193abef 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -324,6 +324,7 @@ static int vhost_user_blk_connect(DeviceState *dev, Error
> **errp)
> }
> s->connected = true;
>
> +s->dev.num_queues = s->num_queues;
> s->dev.nvqs = s->num_queues;
> s->dev.vqs = s->vhost_vqs;
> s->dev.vq_index = 0;
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index ded0c10453..ee57abe045 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -1909,6 +1909,11 @@ static int vhost_user_backend_init(struct vhost_dev
> *dev, void *opaque)
> return err;
> }
> }
> +if (dev->num_queues && dev->max_queues < dev->num_queues) {
> +error_report("The maximum number of queues supported by the "
> + "backend is %" PRIu64, dev->max_queues);
> +return -EINVAL;
> +}
>
> if (virtio_has_feature(features, VIRTIO_F_IOMMU_PLATFORM) &&
> !(virtio_has_feature(dev->protocol_features,
> --
> 2.30.2
>
Re: [PATCH v4 08/30] target/mips: Declare mips_env_set_pc() inlined in "internal.h"
On 4/28/21 10:03 AM, Philippe Mathieu-Daudé wrote: Rename set_pc() as mips_env_set_pc(), declare it inlined and use it in cpu.c and op_helper.c. Reported-by: Richard Henderson Signed-off-by: Philippe Mathieu-Daudé --- v4: mips_cpu_set_error_pc -> mips_env_set_pc (rth) --- target/mips/internal.h | 10 ++ target/mips/cpu.c | 8 +--- target/mips/op_helper.c | 16 +++- 3 files changed, 14 insertions(+), 20 deletions(-) Reviewed-by: Richard Henderson r~
[Bug 1926497] Re: dp83932 stops working after a short while
I think you must use a more recent kernel because some bugs have been fixed in QEMU and kernel that need both of them in sync. Could you extract the kernel from your m68k disk image to use it with QEMU "-kernel" and "-initrd" parameters? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1926497 Title: dp83932 stops working after a short while Status in QEMU: New Bug description: Following the instructions here https://wiki.qemu.org/Documentation/Platforms/m68k I was able to successfully install debian. However, running apt-get update stalls after the first 1-2MB. root@debian:~# apt-get update Get:1 http://ftp.ports.debian.org/debian-ports sid InRelease [55.3 kB] Ign:1 http://ftp.ports.debian.org/debian-ports sid InRelease Get:2 http://ftp.ports.debian.org/debian-ports sid/main all Packages [8,735 kB] 18% [2 Packages 2,155 kB/8,735 kB 25%] After running apt-get update. I don't seem to be able to send any packets anymore. ping host lookups fail and a subsequent apt-get update makes no progress. I'm launching qemu with: qemu-system-m68k -boot c \ -M q800 -serial none -serial mon:stdio -m 1000M \ -net nic,model=dp83932 -net user \ -append "root=/dev/sda2 rw console=ttyS0 console=tty" \ -kernel vmlinux-4.16.0-1-m68k \ -initrd initrd.img-4.16.0-1-m68k \ -drive file=m68k-deb10.qcow2,format=qcow2 \ -nographic I see this with qemu v6.0.0-rc5 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1926497/+subscriptions
[PATCH] meson: change buildtype when debug_info=no
Meson defaults builds to 'debugoptimized' which adds '-g -O2' to CFLAGS. If the user specifies '--disable-debug-info' we should instead build with 'release' which does not emit any debug info. Signed-off-by: Joelle van Dyne --- configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index 4f374b4889..5c3568cbc3 100755 --- a/configure +++ b/configure @@ -6398,6 +6398,7 @@ NINJA=$ninja $meson setup \ --sysconfdir "$sysconfdir" \ --localedir "$localedir" \ --localstatedir "$local_statedir" \ +--buildtype $(if test "$debug_info" = yes; then echo "debugoptimized"; else echo "release"; fi) \ -Ddocdir="$docdir" \ -Dqemu_firmwarepath="$firmwarepath" \ -Dqemu_suffix="$qemu_suffix" \ -- 2.28.0
Re: [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
On 4/28/21 9:24 PM, Richard Henderson wrote: > On 4/28/21 10:56 AM, Philippe Mathieu-Daudé wrote: >> Are you suggesting to remove the 'if have_block' check? This makes build >> a the files pointlessly for user-mode-only builds... > > But since the objects are not included in the binary, I don't care. > > The build system is already too complex, and building a couple of extra > small files makes only milliseconds of difference. Maybe for libqemuutil.a (this does make a difference with the Python QAPI generated files - another series). I'll wait if we get to a consensus about what exactly is virtiofsd, then revisit this series. Thanks! Phil.
[PATCH v2 13/15] linux-user/s390x: Add build asserts for sigset sizes
At point of usage, it's not immediately obvious that we don't need a loop to copy these arrays. Signed-off-by: Richard Henderson --- linux-user/s390x/signal.c | 5 + 1 file changed, 5 insertions(+) diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c index 81ba59b46a..839a7ae4b3 100644 --- a/linux-user/s390x/signal.c +++ b/linux-user/s390x/signal.c @@ -141,6 +141,8 @@ void setup_frame(int sig, struct target_sigaction *ka, return; } +/* Make sure that we're initializing all of oldmask. */ +QEMU_BUILD_BUG_ON(ARRAY_SIZE(frame->sc.oldmask) != 1); __put_user(set->sig[0], &frame->sc.oldmask[0]); save_sigregs(env, &frame->sregs); @@ -266,6 +268,9 @@ long do_sigreturn(CPUS390XState *env) force_sig(TARGET_SIGSEGV); return -TARGET_QEMU_ESIGRETURN; } + +/* Make sure that we're initializing all of target_set. */ +QEMU_BUILD_BUG_ON(ARRAY_SIZE(target_set.sig) != 1); __get_user(target_set.sig[0], &frame->sc.oldmask[0]); target_to_host_sigset_internal(&set, &target_set); -- 2.25.1
[PATCH v2 15/15] linux-user/s390x: Handle vector regs in signal stack
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 62 +--
1 file changed, 60 insertions(+), 2 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 9d470e4ca0..b537646e60 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -50,6 +50,12 @@ typedef struct {
target_s390_fp_regs fpregs;
} target_sigregs;
+typedef struct {
+uint64_t vxrs_low[16];
+uint64_t vxrs_high[16][2];
+uint8_t reserved[128];
+} target_sigregs_ext;
+
typedef struct {
abi_ulong oldmask[_SIGCONTEXT_NSIG_WORDS];
abi_ulong sregs;
@@ -60,15 +66,20 @@ typedef struct {
target_sigcontext sc;
target_sigregs sregs;
int signo;
+target_sigregs_ext sregs_ext;
uint16_t retcode;
} sigframe;
+#define TARGET_UC_VXRS 2
+
struct target_ucontext {
abi_ulong tuc_flags;
abi_ulong tuc_link;
target_stack_t tuc_stack;
target_sigregs tuc_mcontext;
-target_sigset_t tuc_sigmask; /* mask last for extensibility */
+target_sigset_t tuc_sigmask;
+uint8_t reserved[128 - sizeof(target_sigset_t)];
+target_sigregs_ext tuc_mcontext_ext;
};
typedef struct {
@@ -128,6 +139,24 @@ static void save_sigregs(CPUS390XState *env,
target_sigregs *sregs)
}
}
+static void save_sigregs_ext(CPUS390XState *env, target_sigregs_ext *ext)
+{
+int i;
+
+/*
+ * if (MACHINE_HAS_VX) ...
+ * That said, we always allocate the stack storage and the
+ * space is always available in env.
+ */
+for (i = 0; i < 16; ++i) {
+ __put_user(env->vregs[i][1], &ext->vxrs_low[i]);
+}
+for (i = 0; i < 16; ++i) {
+ __put_user(env->vregs[i + 16][0], &ext->vxrs_high[i][0]);
+ __put_user(env->vregs[i + 16][1], &ext->vxrs_high[i][1]);
+}
+}
+
void setup_frame(int sig, struct target_sigaction *ka,
target_sigset_t *set, CPUS390XState *env)
{
@@ -161,6 +190,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
*/
__put_user(sig, &frame->signo);
+/* Create sigregs_ext on the signal stack. */
+save_sigregs_ext(env, &frame->sregs_ext);
+
/*
* Set up to return from userspace.
* If provided, use a stub already in userspace.
@@ -202,6 +234,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
rt_sigframe *frame;
abi_ulong frame_addr;
abi_ulong restorer;
+abi_ulong uc_flags;
frame_addr = get_sigframe(ka, env, sizeof *frame);
trace_user_setup_rt_frame(env, frame_addr);
@@ -229,10 +262,15 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
tswap_siginfo(&frame->info, info);
/* Create ucontext on the signal stack. */
-__put_user(0, &frame->uc.tuc_flags);
+uc_flags = 0;
+if (s390_has_feat(S390_FEAT_VECTOR)) {
+uc_flags |= TARGET_UC_VXRS;
+}
+__put_user(uc_flags, &frame->uc.tuc_flags);
__put_user(0, &frame->uc.tuc_link);
target_save_altstack(&frame->uc.tuc_stack, env);
save_sigregs(env, &frame->uc.tuc_mcontext);
+save_sigregs_ext(env, &frame->uc.tuc_mcontext_ext);
tswap_sigset(&frame->uc.tuc_sigmask, set);
/* Set up registers for signal handler */
@@ -271,6 +309,24 @@ static void restore_sigregs(CPUS390XState *env,
target_sigregs *sc)
}
}
+static void restore_sigregs_ext(CPUS390XState *env, target_sigregs_ext *ext)
+{
+int i;
+
+/*
+ * if (MACHINE_HAS_VX) ...
+ * That said, we always allocate the stack storage and the
+ * space is always available in env.
+ */
+for (i = 0; i < 16; ++i) {
+ __get_user(env->vregs[i][1], &ext->vxrs_low[i]);
+}
+for (i = 0; i < 16; ++i) {
+ __get_user(env->vregs[i + 16][0], &ext->vxrs_high[i][0]);
+ __get_user(env->vregs[i + 16][1], &ext->vxrs_high[i][1]);
+}
+}
+
long do_sigreturn(CPUS390XState *env)
{
sigframe *frame;
@@ -292,6 +348,7 @@ long do_sigreturn(CPUS390XState *env)
set_sigmask(&set); /* ~_BLOCKABLE? */
restore_sigregs(env, &frame->sregs);
+restore_sigregs_ext(env, &frame->sregs_ext);
unlock_user_struct(frame, frame_addr, 0);
return -TARGET_QEMU_ESIGRETURN;
@@ -313,6 +370,7 @@ long do_rt_sigreturn(CPUS390XState *env)
set_sigmask(&set); /* ~_BLOCKABLE? */
restore_sigregs(env, &frame->uc.tuc_mcontext);
+restore_sigregs_ext(env, &frame->uc.tuc_mcontext_ext);
target_restore_altstack(&frame->uc.tuc_stack, env);
--
2.25.1
[PATCH v2 11/15] linux-user/s390x: Add stub sigframe argument for last_break
In order to properly present these arguments, we need to add code to target/s390x to record LowCore parameters for user-only. But in the meantime, at least zero the missing last_break argument, and fixup the comment style in the vicinity. Signed-off-by: Richard Henderson --- linux-user/s390x/signal.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c index 17f617c655..bc41b01c5d 100644 --- a/linux-user/s390x/signal.c +++ b/linux-user/s390x/signal.c @@ -167,13 +167,16 @@ void setup_frame(int sig, struct target_sigaction *ka, | (env->psw.mask & ~PSW_MASK_ASC); env->psw.addr = ka->_sa_handler; -env->regs[2] = sig; //map_signal(sig); +env->regs[2] = sig; env->regs[3] = frame_addr += offsetof(typeof(*frame), sc); -/* We forgot to include these in the sigcontext. - To avoid breaking binary compatibility, they are passed as args. */ -env->regs[4] = 0; // FIXME: no clue... current->thread.trap_no; -env->regs[5] = 0; // FIXME: no clue... current->thread.prot_addr; +/* + * We forgot to include these in the sigcontext. + * To avoid breaking binary compatibility, they are passed as args. + */ +env->regs[4] = 0; /* FIXME: regs->int_code & 127 */ +env->regs[5] = 0; /* FIXME: regs->int_parm_long */ +env->regs[6] = 0; /* FIXME: current->thread.last_break */ /* Place signal number on stack to allow backtrace from handler. */ __put_user(env->regs[2], &frame->signo); @@ -223,9 +226,10 @@ void setup_rt_frame(int sig, struct target_sigaction *ka, | (env->psw.mask & ~PSW_MASK_ASC); env->psw.addr = ka->_sa_handler; -env->regs[2] = sig; //map_signal(sig); +env->regs[2] = sig; env->regs[3] = frame_addr + offsetof(typeof(*frame), info); env->regs[4] = frame_addr + offsetof(typeof(*frame), uc); +env->regs[5] = 0; /* FIXME: current->thread.last_break */ } static void restore_sigregs(CPUS390XState *env, target_sigregs *sc) -- 2.25.1
RE: [RFC PATCH 3/4] target/ppc: Move SPR generation to separate file
Bruno Piazera Larsen writes: >> > > +/*/ >> > > +/* SPR definitions and registration */ >> > > + >> > > +#ifdef CONFIG_TCG >> > > +#ifdef CONFIG_USER_ONLY >> > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, one_reg_id, >> > > initial_value) \ >> > > +_spr_register(env, num, name, uea_read, uea_write, initial_value) >> > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, >> > >\ >> > > +oea_read, oea_write, hea_read, hea_write, >> > >\ >> > > +one_reg_id, initial_value) >> > >\ >> > > +_spr_register(env, num, name, uea_read, uea_write, initial_value) >> > > +#else /* CONFIG_USER_ONLY */ >> > > +#if !defined(CONFIG_KVM) >> > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, one_reg_id, >> > > initial_value) \ >> > > +_spr_register(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, oea_read, oea_write, >> > > initial_value) >> > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, >> > >\ >> > > +oea_read, oea_write, hea_read, hea_write, >> > >\ >> > > +one_reg_id, initial_value) >> > >\ >> > > +_spr_register(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, hea_read, hea_write, >> > > initial_value) >> > > +#else /* !CONFIG_KVM */ >> > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, one_reg_id, >> > > initial_value) \ >> > > +_spr_register(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, oea_read, oea_write, >> > >\ >> > > + one_reg_id, initial_value) >> > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, >> > >\ >> > > +oea_read, oea_write, hea_read, hea_write, >> > >\ >> > > +one_reg_id, initial_value) >> > >\ >> > > +_spr_register(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, hea_read, hea_write, >> > >\ >> > > + one_reg_id, initial_value) >> > > +#endif /* !CONFIG_KVM */ >> > > +#endif /* CONFIG_USER_ONLY */ >> > > +#else /* CONFIG_TCG */ >> > > +#ifdef CONFIG_KVM /* sanity check */ >> > > +#define spr_register_kvm(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, one_reg_id, >> > > initial_value) \ >> > > +_spr_register(env, num, name, one_reg_id, initial_value) >> > > +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write, >> > >\ >> > > +oea_read, oea_write, hea_read, hea_write, >> > >\ >> > > +one_reg_id, initial_value) >> > >\ >> > > +_spr_register(env, num, name, one_reg_id, initial_value) >> > > +#else /* CONFIG_KVM */ >> > > +#error "Either TCG or KVM should be configured" >> > > +#endif /* CONFIG_KVM */ >> > > +#endif /*CONFIG_TCG */ >> > > + >> > > +#define spr_register(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, initial_value) >> > >\ >> > > +spr_register_kvm(env, num, name, uea_read, uea_write, >> > >\ >> > > + oea_read, oea_write, 0, initial_value) >> > > + >> > > +#define spr_register_hv(env, num, name, uea_read, uea_write, >> > >\ >> > > +oea_read, oea_write, hea_read, hea_write, >> > >\ >> > > +initial_value) >> > >\ >> > > +spr_register_kvm_hv(env, num, name, uea_read, uea_write, >> > >\ >> > > +oea_read, oea_write, hea_read, hea_write, >> > >\ >> > > +0, initial_value) >> > >> > This change is crucial for this to work, we don't want it buried along >> > with all of the code movement. It should be clearly described and in >> > it's own patch at the top of the series. >> > >> > If you add this change, plus some #ifdef TCG around the spr callbacks, >> > it should already be possible to compile this with disable-tcg.
Re: [PATCH v4 00/36] block: update graph permissions update
28.04.2021 20:03, Kevin Wolf wrote: Am 28.04.2021 um 17:17 hat Vladimir Sementsov-Ogievskiy geschrieben: Hi all! And here is v4. Thanks, applied to the block branch. Thanks! And thanks a lot for reviewing! Though the error message shown by the test in patch 18 does need some improvement on top. We should probably name both conflicting users and who blocks whom in a way that is neutral as to which user is the new one. I'll look at it next week. Also, curious that again patch 7 (and only patch 7) got From: mangled by the mailing list. Seems there is something in it that Mailman doesn't like? Very interesting.. Curly braces in subject maybe? But I think, that not a first time I use them.. -- Best regards, Vladimir
[PATCH v2 09/15] linux-user/s390x: Clean up single-use gotos in signal.c
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 29 -
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index eabfe4293f..64a9eab097 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -137,7 +137,8 @@ void setup_frame(int sig, struct target_sigaction *ka,
frame_addr = get_sigframe(ka, env, sizeof(*frame));
trace_user_setup_frame(env, frame_addr);
if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
-goto give_sigsegv;
+force_sigsegv(sig);
+return;
}
__put_user(set->sig[0], &frame->sc.oldmask[0]);
@@ -174,10 +175,6 @@ void setup_frame(int sig, struct target_sigaction *ka,
/* Place signal number on stack to allow backtrace from handler. */
__put_user(env->regs[2], &frame->signo);
unlock_user_struct(frame, frame_addr, 1);
-return;
-
-give_sigsegv:
-force_sigsegv(sig);
}
void setup_rt_frame(int sig, struct target_sigaction *ka,
@@ -190,7 +187,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
frame_addr = get_sigframe(ka, env, sizeof *frame);
trace_user_setup_rt_frame(env, frame_addr);
if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
-goto give_sigsegv;
+force_sigsegv(sig);
+return;
}
tswap_siginfo(&frame->info, info);
@@ -222,10 +220,6 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
env->regs[2] = sig; //map_signal(sig);
env->regs[3] = frame_addr + offsetof(typeof(*frame), info);
env->regs[4] = frame_addr + offsetof(typeof(*frame), uc);
-return;
-
-give_sigsegv:
-force_sigsegv(sig);
}
static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
@@ -259,7 +253,8 @@ long do_sigreturn(CPUS390XState *env)
trace_user_do_sigreturn(env, frame_addr);
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1)) {
-goto badframe;
+force_sig(TARGET_SIGSEGV);
+return -TARGET_QEMU_ESIGRETURN;
}
__get_user(target_set.sig[0], &frame->sc.oldmask[0]);
@@ -270,10 +265,6 @@ long do_sigreturn(CPUS390XState *env)
unlock_user_struct(frame, frame_addr, 0);
return -TARGET_QEMU_ESIGRETURN;
-
-badframe:
-force_sig(TARGET_SIGSEGV);
-return -TARGET_QEMU_ESIGRETURN;
}
long do_rt_sigreturn(CPUS390XState *env)
@@ -284,7 +275,8 @@ long do_rt_sigreturn(CPUS390XState *env)
trace_user_do_rt_sigreturn(env, frame_addr);
if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1)) {
-goto badframe;
+force_sig(TARGET_SIGSEGV);
+return -TARGET_QEMU_ESIGRETURN;
}
target_to_host_sigset(&set, &frame->uc.tuc_sigmask);
@@ -296,9 +288,4 @@ long do_rt_sigreturn(CPUS390XState *env)
unlock_user_struct(frame, frame_addr, 0);
return -TARGET_QEMU_ESIGRETURN;
-
-badframe:
-unlock_user_struct(frame, frame_addr, 0);
-force_sig(TARGET_SIGSEGV);
-return -TARGET_QEMU_ESIGRETURN;
}
--
2.25.1
[PATCH v2 10/15] linux-user/s390x: Set psw.mask properly for the signal handler
Note that PSW_ADDR_{64,32} are called PSW_MASK_{EA,BA}
in the kernel source.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 64a9eab097..17f617c655 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -162,6 +162,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
/* Set up registers for signal handler */
env->regs[15] = frame_addr;
+/* Force default amode and default user address space control. */
+env->psw.mask = PSW_MASK_64 | PSW_MASK_32 | PSW_ASC_PRIMARY
+ | (env->psw.mask & ~PSW_MASK_ASC);
env->psw.addr = ka->_sa_handler;
env->regs[2] = sig; //map_signal(sig);
@@ -215,6 +218,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
/* Set up registers for signal handler */
env->regs[15] = frame_addr;
+/* Force default amode and default user address space control. */
+env->psw.mask = PSW_MASK_64 | PSW_MASK_32 | PSW_ASC_PRIMARY
+ | (env->psw.mask & ~PSW_MASK_ASC);
env->psw.addr = ka->_sa_handler;
env->regs[2] = sig; //map_signal(sig);
--
2.25.1
Re: [PATCH] make vfio and DAX cache work together
On Wed, 28 Apr 2021 20:17:23 +0100
"Dr. David Alan Gilbert" wrote:
> * Dev Audsin (dev.devaq...@gmail.com) wrote:
> > Thanks Dave for your explanation.
> > Any suggestions on how to make VFIO not attempt to map into the
> > unaccessible and unallocated RAM.
>
> I'm not sure;:
>
> static bool vfio_listener_skipped_section(MemoryRegionSection *section)
> {
> return (!memory_region_is_ram(section->mr) &&
> !memory_region_is_iommu(section->mr)) ||
>section->offset_within_address_space & (1ULL << 63);
> }
>
> I'm declaring that region with memory_region_init_ram_ptr; should I be?
> it's not quite like RAM.
> But then I *do* want a kvm slot for it, and I do want it to be accessed
> by mapping rather htan calling IO functions; that makes me think mr->ram
> has to be true.
> But then do we need to add another flag to memory-regions; if we do,
> what is it;
>a) We don't want an 'is_virtio_fs' - it needs to be more generic
>b) 'no_vfio' also feels wrong
>
> Is perhaps 'not_lockable' the right thing to call it?
This reasoning just seems to lead back to "it doesn't work, therefore
don't do it" rather than identifying the property of the region that
makes it safe not to map it for device DMA (assuming that's actually
the case). It's clearly "RAM" as far as QEMU is concerned given how
it's created, but does it actually appear in the VM as generic physical
RAM that the guest OS could program to the device as a DMA target? If
not, what property makes that so, create a flag for that. Thanks,
Alex
[PATCH v2 07/15] linux-user/s390x: Use tswap_sigset in setup_rt_frame
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index f8515dd332..4dde55d4d5 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -182,7 +182,6 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
target_siginfo_t *info,
target_sigset_t *set, CPUS390XState *env)
{
-int i;
rt_sigframe *frame;
abi_ulong frame_addr;
@@ -199,10 +198,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
__put_user((abi_ulong)0, (abi_ulong *)&frame->uc.tuc_link);
target_save_altstack(&frame->uc.tuc_stack, env);
save_sigregs(env, &frame->uc.tuc_mcontext);
-for (i = 0; i < TARGET_NSIG_WORDS; i++) {
-__put_user((abi_ulong)set->sig[i],
- (abi_ulong *)&frame->uc.tuc_sigmask.sig[i]);
-}
+tswap_sigset(&frame->uc.tuc_sigmask, set);
/* Set up to return from userspace. If provided, use a stub
already in userspace. */
--
2.25.1
[PATCH v2 08/15] linux-user/s390x: Tidy save_sigregs
The "save" routines copied from the kernel, which are currently
commented out, are unnecessary in qemu. We can copy from env
where the kernel needs special instructions. Fix comment style.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 4dde55d4d5..eabfe4293f 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -104,23 +104,25 @@ get_sigframe(struct target_sigaction *ka, CPUS390XState
*env, size_t frame_size)
static void save_sigregs(CPUS390XState *env, target_sigregs *sregs)
{
int i;
-//save_access_regs(current->thread.acrs); FIXME
-/* Copy a 'clean' PSW mask to the user to avoid leaking
- information about whether PER is currently on. */
+/*
+ * Copy a 'clean' PSW mask to the user to avoid leaking
+ * information about whether PER is currently on.
+ */
__put_user(env->psw.mask, &sregs->regs.psw.mask);
__put_user(env->psw.addr, &sregs->regs.psw.addr);
+
for (i = 0; i < 16; i++) {
__put_user(env->regs[i], &sregs->regs.gprs[i]);
}
for (i = 0; i < 16; i++) {
__put_user(env->aregs[i], &sregs->regs.acrs[i]);
}
+
/*
* We have to store the fp registers to current->thread.fp_regs
* to merge them with the emulated registers.
*/
-//save_fp_regs(¤t->thread.fp_regs); FIXME
for (i = 0; i < 16; i++) {
__put_user(*get_freg(env, i), &sregs->fpregs.fprs[i]);
}
--
2.25.1
Re: [PATCH 1/5] vhost-user-blk: Don't reconnect during initialisation
On 4/22/21 7:02 PM, Kevin Wolf wrote: > This is a partial revert of commits 77542d43149 and bc79c87bcde. > > Usually, an error during initialisation means that the configuration was > wrong. Reconnecting won't make the error go away, but just turn the > error condition into an endless loop. Avoid this and return errors > again. > > Additionally, calling vhost_user_blk_disconnect() from the chardev event > handler could result in use-after-free because none of the > initialisation code expects that the device could just go away in the TIL initialisation wording. > middle. So removing the call fixes crashes in several places. > > For example, using a num-queues setting that is incompatible with the > backend would result in a crash like this (dereferencing dev->opaque, > which is already NULL): > > #0 0x55d0a4bd in vhost_user_read_cb (source=0x568f4690, > condition=(G_IO_IN | G_IO_HUP), opaque=0x7fffcbf0) at > ../hw/virtio/vhost-user.c:313 > #1 0x55d950d3 in qio_channel_fd_source_dispatch > (source=0x57c3f750, callback=0x55d0a478 , > user_data=0x7fffcbf0) at ../io/channel-watch.c:84 > #2 0x77b32a9f in g_main_context_dispatch () at > /lib64/libglib-2.0.so.0 > #3 0x77b84a98 in g_main_context_iterate.constprop () at > /lib64/libglib-2.0.so.0 > #4 0x77b32163 in g_main_loop_run () at /lib64/libglib-2.0.so.0 > #5 0x55d0a724 in vhost_user_read (dev=0x57bc62f8, > msg=0x7fffcc50) at ../hw/virtio/vhost-user.c:402 > #6 0x55d0ee6b in vhost_user_get_config (dev=0x57bc62f8, > config=0x57bc62ac "", config_len=60) at ../hw/virtio/vhost-user.c:2133 > #7 0x55d56d46 in vhost_dev_get_config (hdev=0x57bc62f8, > config=0x57bc62ac "", config_len=60) at ../hw/virtio/vhost.c:1566 > #8 0x55cdd150 in vhost_user_blk_device_realize (dev=0x57bc60b0, > errp=0x7fffcf90) at ../hw/block/vhost-user-blk.c:510 > #9 0x55d08f6d in virtio_device_realize (dev=0x57bc60b0, > errp=0x7fffcff0) at ../hw/virtio/virtio.c:3660 > > Signed-off-by: Kevin Wolf > --- > hw/block/vhost-user-blk.c | 54 ++- > 1 file changed, 13 insertions(+), 41 deletions(-)
Re: [PATCH v2 00/15] linux-user/s390x: some signal fixes
On 4/28/21 12:33 PM, Richard Henderson wrote: Version 2 splits lazy do-it-all patch. Patch 1 has an additional fix, so I dropped the r-b. ... and I realized as I hit send that this depends on the target_restore_altstack cleanup that's part of https://patchew.org/QEMU/20210426025334.1168495-1-richard.hender...@linaro.org/ For avoidance of doubt: https://gitlab.com/rth7680/qemu/-/tree/fix-signals r~
[PATCH v2 05/15] linux-user/s390x: Fix trace in restore_regs
Directly reading sc->regs.psw.addr misses the bswap
that may be performed by __get_user.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index e455a9818d..dcc6f7bc02 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -232,16 +232,17 @@ give_sigsegv:
static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
{
+target_ulong prev_addr;
int i;
for (i = 0; i < 16; i++) {
__get_user(env->regs[i], &sc->regs.gprs[i]);
}
+prev_addr = env->psw.addr;
__get_user(env->psw.mask, &sc->regs.psw.mask);
-trace_user_s390x_restore_sigregs(env, (unsigned long
long)sc->regs.psw.addr,
- (unsigned long long)env->psw.addr);
__get_user(env->psw.addr, &sc->regs.psw.addr);
+trace_user_s390x_restore_sigregs(env, env->psw.addr, prev_addr);
for (i = 0; i < 16; i++) {
__get_user(env->aregs[i], &sc->regs.acrs[i]);
--
2.25.1
[PATCH v2 03/15] linux-user/s390x: Remove PSW_ADDR_AMODE
This is an unnecessary complication since we only
support 64-bit mode.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index fece8ab97b..1dfca71fa9 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -31,7 +31,6 @@
#define _SIGCONTEXT_NSIG_BPW64 /* FIXME: 31-bit mode -> 32 */
#define _SIGCONTEXT_NSIG_WORDS (_SIGCONTEXT_NSIG / _SIGCONTEXT_NSIG_BPW)
#define _SIGMASK_COPY_SIZE(sizeof(unsigned long)*_SIGCONTEXT_NSIG_WORDS)
-#define PSW_ADDR_AMODE0xUL /* 0x8000UL for
31-bit */
#define S390_SYSCALL_OPCODE ((uint16_t)0x0a00)
typedef struct {
@@ -148,11 +147,9 @@ void setup_frame(int sig, struct target_sigaction *ka,
/* Set up to return from userspace. If provided, use a stub
already in userspace. */
if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = (unsigned long)
-ka->sa_restorer | PSW_ADDR_AMODE;
+env->regs[14] = ka->sa_restorer;
} else {
-env->regs[14] = (frame_addr + offsetof(sigframe, retcode))
-| PSW_ADDR_AMODE;
+env->regs[14] = frame_addr + offsetof(sigframe, retcode);
__put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
&frame->retcode);
}
@@ -162,7 +159,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
/* Set up registers for signal handler */
env->regs[15] = frame_addr;
-env->psw.addr = (target_ulong) ka->_sa_handler | PSW_ADDR_AMODE;
+env->psw.addr = ka->_sa_handler;
env->regs[2] = sig; //map_signal(sig);
env->regs[3] = frame_addr += offsetof(typeof(*frame), sc);
@@ -210,10 +207,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
/* Set up to return from userspace. If provided, use a stub
already in userspace. */
if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = ka->sa_restorer | PSW_ADDR_AMODE;
+env->regs[14] = ka->sa_restorer;
} else {
-env->regs[14] = (frame_addr + offsetof(typeof(*frame), retcode))
-| PSW_ADDR_AMODE;
+env->regs[14] = frame_addr + offsetof(typeof(*frame), retcode);
__put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
&frame->retcode);
}
@@ -223,7 +219,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
/* Set up registers for signal handler */
env->regs[15] = frame_addr;
-env->psw.addr = (target_ulong) ka->_sa_handler | PSW_ADDR_AMODE;
+env->psw.addr = ka->_sa_handler;
env->regs[2] = sig; //map_signal(sig);
env->regs[3] = frame_addr + offsetof(typeof(*frame), info);
@@ -248,7 +244,6 @@ restore_sigregs(CPUS390XState *env, target_sigregs *sc)
trace_user_s390x_restore_sigregs(env, (unsigned long
long)sc->regs.psw.addr,
(unsigned long long)env->psw.addr);
__get_user(env->psw.addr, &sc->regs.psw.addr);
-/* FIXME: 31-bit -> | PSW_ADDR_AMODE */
for (i = 0; i < 16; i++) {
__get_user(env->aregs[i], &sc->regs.acrs[i]);
--
2.25.1
[Bug 1926497] Re: dp83932 stops working after a short while
** Tags added: m68k q800 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1926497 Title: dp83932 stops working after a short while Status in QEMU: New Bug description: Following the instructions here https://wiki.qemu.org/Documentation/Platforms/m68k I was able to successfully install debian. However, running apt-get update stalls after the first 1-2MB. root@debian:~# apt-get update Get:1 http://ftp.ports.debian.org/debian-ports sid InRelease [55.3 kB] Ign:1 http://ftp.ports.debian.org/debian-ports sid InRelease Get:2 http://ftp.ports.debian.org/debian-ports sid/main all Packages [8,735 kB] 18% [2 Packages 2,155 kB/8,735 kB 25%] After running apt-get update. I don't seem to be able to send any packets anymore. ping host lookups fail and a subsequent apt-get update makes no progress. I'm launching qemu with: qemu-system-m68k -boot c \ -M q800 -serial none -serial mon:stdio -m 1000M \ -net nic,model=dp83932 -net user \ -append "root=/dev/sda2 rw console=ttyS0 console=tty" \ -kernel vmlinux-4.16.0-1-m68k \ -initrd initrd.img-4.16.0-1-m68k \ -drive file=m68k-deb10.qcow2,format=qcow2 \ -nographic I see this with qemu v6.0.0-rc5 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1926497/+subscriptions
[Bug 1926497] Re: dp83932 stops working after a short while
I also see the same problem with version 4.2.1 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1926497 Title: dp83932 stops working after a short while Status in QEMU: New Bug description: Following the instructions here https://wiki.qemu.org/Documentation/Platforms/m68k I was able to successfully install debian. However, running apt-get update stalls after the first 1-2MB. root@debian:~# apt-get update Get:1 http://ftp.ports.debian.org/debian-ports sid InRelease [55.3 kB] Ign:1 http://ftp.ports.debian.org/debian-ports sid InRelease Get:2 http://ftp.ports.debian.org/debian-ports sid/main all Packages [8,735 kB] 18% [2 Packages 2,155 kB/8,735 kB 25%] After running apt-get update. I don't seem to be able to send any packets anymore. ping host lookups fail and a subsequent apt-get update makes no progress. I'm launching qemu with: qemu-system-m68k -boot c \ -M q800 -serial none -serial mon:stdio -m 1000M \ -net nic,model=dp83932 -net user \ -append "root=/dev/sda2 rw console=ttyS0 console=tty" \ -kernel vmlinux-4.16.0-1-m68k \ -initrd initrd.img-4.16.0-1-m68k \ -drive file=m68k-deb10.qcow2,format=qcow2 \ -nographic I see this with qemu v6.0.0-rc5 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1926497/+subscriptions
[PATCH v2 04/15] linux-user/s390x: Remove restore_sigregs return value
The function cannot fail.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 14 +++---
1 file changed, 3 insertions(+), 11 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 1dfca71fa9..e455a9818d 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -230,10 +230,8 @@ give_sigsegv:
force_sigsegv(sig);
}
-static int
-restore_sigregs(CPUS390XState *env, target_sigregs *sc)
+static void restore_sigregs(CPUS390XState *env, target_sigregs *sc)
{
-int err = 0;
int i;
for (i = 0; i < 16; i++) {
@@ -251,8 +249,6 @@ restore_sigregs(CPUS390XState *env, target_sigregs *sc)
for (i = 0; i < 16; i++) {
__get_user(*get_freg(env, i), &sc->fpregs.fprs[i]);
}
-
-return err;
}
long do_sigreturn(CPUS390XState *env)
@@ -271,9 +267,7 @@ long do_sigreturn(CPUS390XState *env)
target_to_host_sigset_internal(&set, &target_set);
set_sigmask(&set); /* ~_BLOCKABLE? */
-if (restore_sigregs(env, &frame->sregs)) {
-goto badframe;
-}
+restore_sigregs(env, &frame->sregs);
unlock_user_struct(frame, frame_addr, 0);
return -TARGET_QEMU_ESIGRETURN;
@@ -297,9 +291,7 @@ long do_rt_sigreturn(CPUS390XState *env)
set_sigmask(&set); /* ~_BLOCKABLE? */
-if (restore_sigregs(env, &frame->uc.tuc_mcontext)) {
-goto badframe;
-}
+restore_sigregs(env, &frame->uc.tuc_mcontext);
target_restore_altstack(&frame->uc.tuc_stack, env);
--
2.25.1
[PATCH v2 00/15] linux-user/s390x: some signal fixes
Version 2 splits lazy do-it-all patch. Patch 1 has an additional fix, so I dropped the r-b. r~ Richard Henderson (15): linux-user/s390x: Fix sigframe types linux-user/s390x: Use uint16_t for signal retcode linux-user/s390x: Remove PSW_ADDR_AMODE linux-user/s390x: Remove restore_sigregs return value linux-user/s390x: Fix trace in restore_regs linux-user/s390x: Fix sigcontext sregs value linux-user/s390x: Use tswap_sigset in setup_rt_frame linux-user/s390x: Tidy save_sigregs linux-user/s390x: Clean up single-use gotos in signal.c linux-user/s390x: Set psw.mask properly for the signal handler linux-user/s390x: Add stub sigframe argument for last_break linux-user/s390x: Fix frame_addr corruption in setup_frame linux-user/s390x: Add build asserts for sigset sizes linux-user/s390x: Clean up signal.c linux-user/s390x: Handle vector regs in signal stack linux-user/s390x/signal.c | 280 +++--- 1 file changed, 170 insertions(+), 110 deletions(-) -- 2.25.1
[Bug 1926497] [NEW] dp83932 stops working after a short while
Public bug reported: Following the instructions here https://wiki.qemu.org/Documentation/Platforms/m68k I was able to successfully install debian. However, running apt-get update stalls after the first 1-2MB. root@debian:~# apt-get update Get:1 http://ftp.ports.debian.org/debian-ports sid InRelease [55.3 kB] Ign:1 http://ftp.ports.debian.org/debian-ports sid InRelease Get:2 http://ftp.ports.debian.org/debian-ports sid/main all Packages [8,735 kB] 18% [2 Packages 2,155 kB/8,735 kB 25%] After running apt-get update. I don't seem to be able to send any packets anymore. ping host lookups fail and a subsequent apt-get update makes no progress. I'm launching qemu with: qemu-system-m68k -boot c \ -M q800 -serial none -serial mon:stdio -m 1000M \ -net nic,model=dp83932 -net user \ -append "root=/dev/sda2 rw console=ttyS0 console=tty" \ -kernel vmlinux-4.16.0-1-m68k \ -initrd initrd.img-4.16.0-1-m68k \ -drive file=m68k-deb10.qcow2,format=qcow2 \ -nographic I see this with qemu v6.0.0-rc5 ** Affects: qemu Importance: Undecided Status: New ** Tags: m68k q800 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1926497 Title: dp83932 stops working after a short while Status in QEMU: New Bug description: Following the instructions here https://wiki.qemu.org/Documentation/Platforms/m68k I was able to successfully install debian. However, running apt-get update stalls after the first 1-2MB. root@debian:~# apt-get update Get:1 http://ftp.ports.debian.org/debian-ports sid InRelease [55.3 kB] Ign:1 http://ftp.ports.debian.org/debian-ports sid InRelease Get:2 http://ftp.ports.debian.org/debian-ports sid/main all Packages [8,735 kB] 18% [2 Packages 2,155 kB/8,735 kB 25%] After running apt-get update. I don't seem to be able to send any packets anymore. ping host lookups fail and a subsequent apt-get update makes no progress. I'm launching qemu with: qemu-system-m68k -boot c \ -M q800 -serial none -serial mon:stdio -m 1000M \ -net nic,model=dp83932 -net user \ -append "root=/dev/sda2 rw console=ttyS0 console=tty" \ -kernel vmlinux-4.16.0-1-m68k \ -initrd initrd.img-4.16.0-1-m68k \ -drive file=m68k-deb10.qcow2,format=qcow2 \ -nographic I see this with qemu v6.0.0-rc5 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1926497/+subscriptions
[PATCH v2 14/15] linux-user/s390x: Clean up signal.c
Reorder the function bodies to correspond to the kernel source.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 67 ---
1 file changed, 41 insertions(+), 26 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 839a7ae4b3..9d470e4ca0 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -133,6 +133,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
{
sigframe *frame;
abi_ulong frame_addr;
+abi_ulong restorer;
frame_addr = get_sigframe(ka, env, sizeof(*frame));
trace_user_setup_frame(env, frame_addr);
@@ -141,28 +142,39 @@ void setup_frame(int sig, struct target_sigaction *ka,
return;
}
+/* Set up backchain. */
+__put_user(env->regs[15], (abi_ulong *) frame);
+
+/* Create struct sigcontext on the signal stack. */
/* Make sure that we're initializing all of oldmask. */
QEMU_BUILD_BUG_ON(ARRAY_SIZE(frame->sc.oldmask) != 1);
__put_user(set->sig[0], &frame->sc.oldmask[0]);
-
-save_sigregs(env, &frame->sregs);
-
__put_user(frame_addr + offsetof(sigframe, sregs), &frame->sc.sregs);
-/* Set up to return from userspace. If provided, use a stub
- already in userspace. */
+/* Create _sigregs on the signal stack */
+save_sigregs(env, &frame->sregs);
+
+/*
+ * ??? The kernel uses regs->gprs[2] here, which is not yet the signo.
+ * Moreover the comment talks about allowing backtrace, which is really
+ * done by the r15 copy above.
+ */
+__put_user(sig, &frame->signo);
+
+/*
+ * Set up to return from userspace.
+ * If provided, use a stub already in userspace.
+ */
if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = ka->sa_restorer;
+restorer = ka->sa_restorer;
} else {
-env->regs[14] = frame_addr + offsetof(sigframe, retcode);
+restorer = frame_addr + offsetof(sigframe, retcode);
__put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
&frame->retcode);
}
-/* Set up backchain. */
-__put_user(env->regs[15], (abi_ulong *) frame);
-
/* Set up registers for signal handler */
+env->regs[14] = restorer;
env->regs[15] = frame_addr;
/* Force default amode and default user address space control. */
env->psw.mask = PSW_MASK_64 | PSW_MASK_32 | PSW_ASC_PRIMARY
@@ -180,8 +192,6 @@ void setup_frame(int sig, struct target_sigaction *ka,
env->regs[5] = 0; /* FIXME: regs->int_parm_long */
env->regs[6] = 0; /* FIXME: current->thread.last_break */
-/* Place signal number on stack to allow backtrace from handler. */
-__put_user(env->regs[2], &frame->signo);
unlock_user_struct(frame, frame_addr, 1);
}
@@ -191,6 +201,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
{
rt_sigframe *frame;
abi_ulong frame_addr;
+abi_ulong restorer;
frame_addr = get_sigframe(ka, env, sizeof *frame);
trace_user_setup_rt_frame(env, frame_addr);
@@ -199,29 +210,33 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
return;
}
-tswap_siginfo(&frame->info, info);
+/* Set up backchain. */
+__put_user(env->regs[15], (abi_ulong *) frame);
-/* Create the ucontext. */
-__put_user(0, &frame->uc.tuc_flags);
-__put_user((abi_ulong)0, (abi_ulong *)&frame->uc.tuc_link);
-target_save_altstack(&frame->uc.tuc_stack, env);
-save_sigregs(env, &frame->uc.tuc_mcontext);
-tswap_sigset(&frame->uc.tuc_sigmask, set);
-
-/* Set up to return from userspace. If provided, use a stub
- already in userspace. */
+/*
+ * Set up to return from userspace.
+ * If provided, use a stub already in userspace.
+ */
if (ka->sa_flags & TARGET_SA_RESTORER) {
-env->regs[14] = ka->sa_restorer;
+restorer = ka->sa_restorer;
} else {
-env->regs[14] = frame_addr + offsetof(typeof(*frame), retcode);
+restorer = frame_addr + offsetof(typeof(*frame), retcode);
__put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
&frame->retcode);
}
-/* Set up backchain. */
-__put_user(env->regs[15], (abi_ulong *) frame);
+/* Create siginfo on the signal stack. */
+tswap_siginfo(&frame->info, info);
+
+/* Create ucontext on the signal stack. */
+__put_user(0, &frame->uc.tuc_flags);
+__put_user(0, &frame->uc.tuc_link);
+target_save_altstack(&frame->uc.tuc_stack, env);
+save_sigregs(env, &frame->uc.tuc_mcontext);
+tswap_sigset(&frame->uc.tuc_sigmask, set);
/* Set up registers for signal handler */
+env->regs[14] = restorer;
env->regs[15] = frame_addr;
/* Force default amode and default user address space control. */
env->psw.mask = PSW_MASK_64 | PSW_MASK_32 | PSW_ASC_PRIMARY
--
2.25.1
[PATCH v2 02/15] linux-user/s390x: Use uint16_t for signal retcode
Using the right type simplifies the frame setup.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index 707fb603d7..fece8ab97b 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -25,7 +25,6 @@
#define __NUM_FPRS 16
#define __NUM_ACRS 16
-#define S390_SYSCALL_SIZE 2
#define __SIGNAL_FRAMESIZE 160 /* FIXME: 31-bit mode -> 96 */
#define _SIGCONTEXT_NSIG64
@@ -62,7 +61,7 @@ typedef struct {
target_sigcontext sc;
target_sigregs sregs;
int signo;
-uint8_t retcode[S390_SYSCALL_SIZE];
+uint16_t retcode;
} sigframe;
struct target_ucontext {
@@ -75,7 +74,7 @@ struct target_ucontext {
typedef struct {
uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
-uint8_t retcode[S390_SYSCALL_SIZE];
+uint16_t retcode;
struct target_siginfo info;
struct target_ucontext uc;
} rt_sigframe;
@@ -155,7 +154,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
env->regs[14] = (frame_addr + offsetof(sigframe, retcode))
| PSW_ADDR_AMODE;
__put_user(S390_SYSCALL_OPCODE | TARGET_NR_sigreturn,
- (uint16_t *)(frame->retcode));
+ &frame->retcode);
}
/* Set up backchain. */
@@ -216,7 +215,7 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
env->regs[14] = (frame_addr + offsetof(typeof(*frame), retcode))
| PSW_ADDR_AMODE;
__put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
- (uint16_t *)(frame->retcode));
+ &frame->retcode);
}
/* Set up backchain. */
--
2.25.1
[PATCH v2 12/15] linux-user/s390x: Fix frame_addr corruption in setup_frame
The original value of frame_addr is still required for its use in the call to unlock_user_struct below. Signed-off-by: Richard Henderson --- linux-user/s390x/signal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c index bc41b01c5d..81ba59b46a 100644 --- a/linux-user/s390x/signal.c +++ b/linux-user/s390x/signal.c @@ -168,7 +168,7 @@ void setup_frame(int sig, struct target_sigaction *ka, env->psw.addr = ka->_sa_handler; env->regs[2] = sig; -env->regs[3] = frame_addr += offsetof(typeof(*frame), sc); +env->regs[3] = frame_addr + offsetof(typeof(*frame), sc); /* * We forgot to include these in the sigcontext. -- 2.25.1
[PATCH v2 06/15] linux-user/s390x: Fix sigcontext sregs value
Using the host address of &frame->sregs is incorrect. We need the guest address. Signed-off-by: Richard Henderson --- linux-user/s390x/signal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c index dcc6f7bc02..f8515dd332 100644 --- a/linux-user/s390x/signal.c +++ b/linux-user/s390x/signal.c @@ -142,7 +142,7 @@ void setup_frame(int sig, struct target_sigaction *ka, save_sigregs(env, &frame->sregs); -__put_user((abi_ulong)(unsigned long)&frame->sregs, &frame->sc.sregs); +__put_user(frame_addr + offsetof(sigframe, sregs), &frame->sc.sregs); /* Set up to return from userspace. If provided, use a stub already in userspace. */ -- 2.25.1
[PATCH v2 01/15] linux-user/s390x: Fix sigframe types
Noticed via gitlab clang-user job:
TESTsignals on s390x
../linux-user/s390x/signal.c:258:9: runtime error: \
1.84467e+19 is outside the range of representable values of \
type 'unsigned long'
Which points to the fact that we were performing a double-to-uint64_t
conversion while storing the fp registers, instead of just copying
the data across.
Turns out there are several errors:
target_ulong is the size of the target register, whereas abi_ulong
is the target 'unsigned long' type. Not a big deal here, since we
only support 64-bit s390x, but not correct either.
In target_sigcontext and target ucontext, we used a host pointer
instead of a target pointer, aka abi_ulong.
Fixing this allows the removal of a cast to __put_user.
Signed-off-by: Richard Henderson
---
linux-user/s390x/signal.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index b68b44ae7e..707fb603d7 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -37,13 +37,14 @@
typedef struct {
target_psw_t psw;
-target_ulong gprs[__NUM_GPRS];
-unsigned int acrs[__NUM_ACRS];
+abi_ulong gprs[__NUM_GPRS];
+abi_uint acrs[__NUM_ACRS];
} target_s390_regs_common;
typedef struct {
-unsigned int fpc;
-double fprs[__NUM_FPRS];
+uint32_t fpc;
+uint32_t pad;
+uint64_t fprs[__NUM_FPRS];
} target_s390_fp_regs;
typedef struct {
@@ -51,22 +52,22 @@ typedef struct {
target_s390_fp_regs fpregs;
} target_sigregs;
-struct target_sigcontext {
-target_ulong oldmask[_SIGCONTEXT_NSIG_WORDS];
-target_sigregs *sregs;
-};
+typedef struct {
+abi_ulong oldmask[_SIGCONTEXT_NSIG_WORDS];
+abi_ulong sregs;
+} target_sigcontext;
typedef struct {
uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
-struct target_sigcontext sc;
+target_sigcontext sc;
target_sigregs sregs;
int signo;
uint8_t retcode[S390_SYSCALL_SIZE];
} sigframe;
struct target_ucontext {
-target_ulong tuc_flags;
-struct target_ucontext *tuc_link;
+abi_ulong tuc_flags;
+abi_ulong tuc_link;
target_stack_t tuc_stack;
target_sigregs tuc_mcontext;
target_sigset_t tuc_sigmask; /* mask last for extensibility */
@@ -143,8 +144,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
save_sigregs(env, &frame->sregs);
-__put_user((abi_ulong)(unsigned long)&frame->sregs,
- (abi_ulong *)&frame->sc.sregs);
+__put_user((abi_ulong)(unsigned long)&frame->sregs, &frame->sc.sregs);
/* Set up to return from userspace. If provided, use a stub
already in userspace. */
--
2.25.1
Re: [PATCH v4] target/ppc: code motion from translate_init.c.inc to gdbstub.c
This is a test. Please disregard
Re: [PATCH 4/5] virtio: Fail if iommu_platform is requested, but unsupported
On Thu, Apr 22, 2021 at 07:02:20PM +0200, Kevin Wolf wrote:
> Commit 2943b53f6 (' virtio: force VIRTIO_F_IOMMU_PLATFORM') made sure
> that vhost can't just reject VIRTIO_F_IOMMU_PLATFORM when it was
> requested. However, just adding it back to the negotiated flags isn't
> right either because it promises support to the guest that the device
> actually doesn't support. One example of a vhost-user device that
> doesn't have support for the flag is the vhost-user-blk export of QEMU.
>
> Instead of successfully creating a device that doesn't work, just fail
> to plug the device when it doesn't support the feature, but it was
> requested. This results in much clearer error messages.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935031
> Signed-off-by: Kevin Wolf
> ---
> hw/virtio/virtio-bus.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index d6332d45c3..859978d248 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -69,6 +69,11 @@ void virtio_bus_device_plugged(VirtIODevice *vdev, Error
> **errp)
> return;
> }
>
Can you explain this check a little more?
Above we have:
bool has_iommu = virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM);
and then we get the host features from the bckend:
vdev->host_features = vdc->get_features(vdev, vdev->host_features
So as is this is catching the case where vdev->host_features had
VIRTIO_F_IOMMU_PLATFORM set before (by default?), but doesn't now that
the features have been retrieved?
Why not just:
if (!virtio_host_has_feature(vdev, VIRTIO_F_IOMMU_PLATFORM)) {
> +if (has_iommu && !virtio_host_has_feature(vdev,
> VIRTIO_F_IOMMU_PLATFORM)) {
> +error_setg(errp, "iommu_platform=true is not supported by the
> device");
> +return;
> +}
> +
> if (klass->device_plugged != NULL) {
> klass->device_plugged(qbus->parent, &local_err);
> }
> --
> 2.30.2
>
Re: [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
On 4/28/21 10:56 AM, Philippe Mathieu-Daudé wrote: Are you suggesting to remove the 'if have_block' check? This makes build a the files pointlessly for user-mode-only builds... But since the objects are not included in the binary, I don't care. The build system is already too complex, and building a couple of extra small files makes only milliseconds of difference. r~
Re: [PATCH] make vfio and DAX cache work together
* Dev Audsin (dev.devaq...@gmail.com) wrote:
> Thanks Dave for your explanation.
> Any suggestions on how to make VFIO not attempt to map into the
> unaccessible and unallocated RAM.
I'm not sure;:
static bool vfio_listener_skipped_section(MemoryRegionSection *section)
{
return (!memory_region_is_ram(section->mr) &&
!memory_region_is_iommu(section->mr)) ||
section->offset_within_address_space & (1ULL << 63);
}
I'm declaring that region with memory_region_init_ram_ptr; should I be?
it's not quite like RAM.
But then I *do* want a kvm slot for it, and I do want it to be accessed
by mapping rather htan calling IO functions; that makes me think mr->ram
has to be true.
But then do we need to add another flag to memory-regions; if we do,
what is it;
a) We don't want an 'is_virtio_fs' - it needs to be more generic
b) 'no_vfio' also feels wrong
Is perhaps 'not_lockable' the right thing to call it?
Dave
> Best
> Dev
>
> On Tue, Apr 27, 2021 at 8:00 PM Dr. David Alan Gilbert
> wrote:
> >
> > * Alex Williamson (alex.william...@redhat.com) wrote:
> > > On Tue, 27 Apr 2021 17:29:37 +0100
> > > Dev Audsin wrote:
> > >
> > > > Hi Alex
> > > >
> > > > Based on your comments and thinking a bit, wonder if it makes sense to
> > > > allow DMA map for the DAX cache but make unexpected mappings to be not
> > > > fatal. Please let me know your thoughts.
> > >
> > > I think you're still working on the assumption that simply making the
> > > VM boot is an improvement, it's not. If there's a risk that a possible
> > > DMA target for the device cannot be mapped, it's better that the VM
> > > fail to boot than to expose that risk. Performance cannot compromise
> > > correctness.
> > >
> > > We do allow DMA mappings to other device memory regions to fail
> > > non-fatally with the logic that peer-to-peer DMA is often not trusted
> > > to work by drivers and therefore support would be probed before
> > > assuming that it works. I don't think that same logic applies here.
> > >
> > > Is there something about the definition of this particular region that
> > > precludes it from being a DMA target for an assigned devices?
> >
> > It's never really the ram that's used.
> > This area is really a chunk of VMA that's mmap'd over by (chunks of)
> > normal files in the underlying exported filesystem. The actual RAM
> > block itself is just a placeholder for the VMA, and is normally mapped
> > PROT_NONE until an actual file is mapped on top of it.
> > That cache bar is a mapping containing multiple separate file chunk
> > mappings.
> >
> > So I guess the problems for VFIO are:
> > a) At the start it's unmapped, unaccessible, unallocated ram.
> > b) Later it's arbitrary chunks of ondisk files.
> >
> > [on a bad day, and it's bad even without vfio, someone truncates the
> > file mapping]
> >
> > Dave
> >
> > > Otherwise if it's initially unpopulated, maybe something like the
> > > RamDiscardManager could be used to insert DMA mappings as the region
> > > becomes populated.
> > >
> > > Simply disabling mapping to boot with both features together, without
> > > analyzing how that missing mapping affects their interaction is not
> > > acceptable. Thanks,
> > >
> > > Alex
> > >
> > > > On Mon, Apr 26, 2021 at 10:22 PM Alex Williamson
> > > > wrote:
> > > > >
> > > > > On Mon, 26 Apr 2021 21:50:38 +0100
> > > > > Dev Audsin wrote:
> > > > >
> > > > > > Hi Alex and David
> > > > > >
> > > > > > @Alex:
> > > > > >
> > > > > > Justification on why this region cannot be a DMA target for the
> > > > > > device,
> > > > > >
> > > > > > virtio-fs with DAX is currently not compatible with NIC Pass
> > > > > > through.
> > > > > > When a SR-IOV VF attaches to a qemu process, vfio will try to pin
> > > > > > the
> > > > > > entire DAX Window but it is empty when the guest boots and will
> > > > > > fail.
> > > > > > A method to make VFIO and DAX to work together is to make vfio skip
> > > > > > DAX cache.
> > > > > >
> > > > > > Currently DAX cache need to be set to 0, for the SR-IOV VF to be
> > > > > > attached to Kata containers. Enabling both SR-IOV VF and DAX work
> > > > > > together will potentially improve performance for workloads which
> > > > > > are
> > > > > > I/O and network intensive.
> > > > >
> > > > > Sorry, there's no actual justification described here. You're
> > > > > enabling
> > > > > a VM with both features, virtio-fs DAX and VFIO, but there's no
> > > > > evidence that they "work together" or that your use case is simply
> > > > > avoiding a scenario where the device might attempt to DMA into the
> > > > > area
> > > > > with this designation. With this change, if the device were to
> > > > > attempt
> > > > > to DMA into this region, it would be blocked by the IOMMU, which might
> > > > > result in a data loss within the VM. Justification of this change
> > > > > needs to prove that this region can never be a DMA target for the
> > > > > device, not simply that both featu
Re: [PATCH] hw/avr/atmega.c: use the avr51 cpu for atmega1280
Cc'ing Joaquín.
On 4/28/21 9:15 PM, Frederic Konrad wrote:
> According to the as documentation:
> (https://sourceware.org/binutils/docs-2.36/as/AVR-Options.html)
>
> "Instruction set avr51 is for the enhanced AVR core with exactly 128K
> program memory space (MCU types: atmega128, atmega128a, atmega1280,
> atmega1281, atmega1284, atmega1284p, atmega128rfa1, atmega128rfr2,
> atmega1284rfr2, at90can128, at90usb1286, at90usb1287, m3000)."
>
> But when compiling a program for atmega1280 or avr51 and trying to execute
> it:
>
> $ cat > test.S << EOF
>> loop:
>> rjmp loop
>> EOF
> $ avr-gcc -nostdlib -nostartfiles -mmcu=atmega1280 test.S -o test.elf
> $ qemu-system-avr -serial mon:stdio -nographic -no-reboot -M mega \
> -bios test.elf
> qemu-system-avr: Current machine: Arduino Mega (ATmega1280) with 'avr6' CPU
> qemu-system-avr: ELF image 'test.elf' is for 'avr51' CPU
>
> So this fixes the atmega1280 class to use an avr51 CPU.
>
> Signed-off-by: Frederic Konrad
> ---
> hw/avr/atmega.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
> index 44c6afebbb..e3ea5702f5 100644
> --- a/hw/avr/atmega.c
> +++ b/hw/avr/atmega.c
> @@ -402,7 +402,7 @@ static void atmega1280_class_init(ObjectClass *oc, void
> *data)
> {
> AtmegaMcuClass *amc = ATMEGA_MCU_CLASS(oc);
>
> -amc->cpu_type = AVR_CPU_TYPE_NAME("avr6");
> +amc->cpu_type = AVR_CPU_TYPE_NAME("avr51");
> amc->flash_size = 128 * KiB;
> amc->eeprom_size = 4 * KiB;
> amc->sram_size = 8 * KiB;
>
[PATCH] hw/avr/atmega.c: use the avr51 cpu for atmega1280
According to the as documentation:
(https://sourceware.org/binutils/docs-2.36/as/AVR-Options.html)
"Instruction set avr51 is for the enhanced AVR core with exactly 128K
program memory space (MCU types: atmega128, atmega128a, atmega1280,
atmega1281, atmega1284, atmega1284p, atmega128rfa1, atmega128rfr2,
atmega1284rfr2, at90can128, at90usb1286, at90usb1287, m3000)."
But when compiling a program for atmega1280 or avr51 and trying to execute
it:
$ cat > test.S << EOF
> loop:
> rjmp loop
> EOF
$ avr-gcc -nostdlib -nostartfiles -mmcu=atmega1280 test.S -o test.elf
$ qemu-system-avr -serial mon:stdio -nographic -no-reboot -M mega \
-bios test.elf
qemu-system-avr: Current machine: Arduino Mega (ATmega1280) with 'avr6' CPU
qemu-system-avr: ELF image 'test.elf' is for 'avr51' CPU
So this fixes the atmega1280 class to use an avr51 CPU.
Signed-off-by: Frederic Konrad
---
hw/avr/atmega.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb..e3ea5702f5 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -402,7 +402,7 @@ static void atmega1280_class_init(ObjectClass *oc, void
*data)
{
AtmegaMcuClass *amc = ATMEGA_MCU_CLASS(oc);
-amc->cpu_type = AVR_CPU_TYPE_NAME("avr6");
+amc->cpu_type = AVR_CPU_TYPE_NAME("avr51");
amc->flash_size = 128 * KiB;
amc->eeprom_size = 4 * KiB;
amc->sram_size = 8 * KiB;
--
2.30.1
Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected
On 4/28/21 9:04 PM, Alex Bennée wrote: > > Philippe Mathieu-Daudé writes: > >> On 4/28/21 7:06 PM, Alex Bennée wrote: >>> Philippe Mathieu-Daudé writes: >>> Alex, Richard, do you mind reviewing this one please? >>> >>> Isn't it already merged (with my r-b tag no less ;-) >>> >>> f77147cd4de8c726f89b2702f7a9d0c9711d8875 >> >> See ... >> >>> Author: Philippe Mathieu-Daudé >>> AuthorDate: Fri Jan 22 21:44:31 2021 +0100 >>> Commit: Paolo Bonzini >>> CommitDate: Mon Feb 8 14:43:55 2021 +0100 >>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote: > From: Philippe Mathieu-Daudé > > The previous attempt (commit f77147cd4de) doesn't work as >> >> ... ^ this comment :( > > Ahh - my tooling was confused having searched by the subject title ;-) Oh I see, I'll rename as: "tests/meson: Only build softfloat objects if TCG is selected (again)".
Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected
Philippe Mathieu-Daudé writes:
> On 4/28/21 7:06 PM, Alex Bennée wrote:
>> Philippe Mathieu-Daudé writes:
>>
>>> Alex, Richard, do you mind reviewing this one please?
>>
>> Isn't it already merged (with my r-b tag no less ;-)
>>
>> f77147cd4de8c726f89b2702f7a9d0c9711d8875
>
> See ...
>
>> Author: Philippe Mathieu-Daudé
>> AuthorDate: Fri Jan 22 21:44:31 2021 +0100
>> Commit: Paolo Bonzini
>> CommitDate: Mon Feb 8 14:43:55 2021 +0100
>>
>>>
>>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
From: Philippe Mathieu-Daudé
The previous attempt (commit f77147cd4de) doesn't work as
>
> ... ^ this comment :(
Ahh - my tooling was confused having searched by the subject title ;-)
>
expected, as we still have CONFIG_TCG=1 when using:
configure --disable-system --disable-user
Now than we have removed the use of CONFIG_TCG from target-dependent
files in tests/qtest/, we can remove the unconditional definition of
CONFIG_TCG in config_host.
This avoid to build a bunch of unrequired objects when building with
--disable-tcg (in particular the softfloat tests):
Before:
$ make
[1/812] Generating trace-qom.h with a custom command
...
After:
$ make
[1/349] Generating trace-qom.h with a custom command
...
A difference of 463 objects...
Reported-by: Claudio Fontana
Suggested-by: Paolo Bonzini
Signed-off-by: Philippe Mathieu-Daudé
---
v3: Include Paolo's feedback:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
therefore o not include Alex's R-b tag.
Cc: Richard Henderson
Cc: Alex Bennée
Cc: Emilio G. Cota
---
meson.build | 1 -
1 file changed, 1 deletion(-)
diff --git a/meson.build b/meson.build
index c6f4b0cf5e8..623cbe50685 100644
--- a/meson.build
+++ b/meson.build
@@ -262,7 +262,6 @@
language: ['c', 'cpp', 'objc'])
accelerators += 'CONFIG_TCG'
- config_host += { 'CONFIG_TCG': 'y' }
endif
if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>>
>>
--
Alex Bennée
Re: [PATCH] i386: Document when features can be added to kvm_default_props
On Fri, Sep 25, 2020 at 05:10:21PM -0400, Eduardo Habkost wrote: > It's very easy to mistakenly extend kvm_default_props to include > features that require a kernel version that's too recent. Add a > comment warning about that, pointing to the documentation file > where the minimum kernel version for KVM is documented. > > Signed-off-by: Eduardo Habkost I forgot about this. I'm queueing it now (7 months later). -- Eduardo
Re: [PATCH 5/7] hw: Have machines Kconfig-select FW_CFG
On Mon, Apr 26, 2021 at 09:35:18PM +0200, Philippe Mathieu-Daudé wrote: > Beside the loongson3-virt machine (MIPS), the following machines > also use the fw_cfg device: > > - ARM: virt & sbsa-ref > - HPPA: generic machine > - X86: ACPI based (pc & microvm) > - PPC64: various > - SPARC: sun4m & sun4u > > Add their FW_CFG Kconfig dependency. > > Signed-off-by: Philippe Mathieu-Daudé Acked-by: Eduardo Habkost (i386) -- Eduardo
Re: [PATCH 3/5] vhost-user-blk: Get more feature flags from vhost devicey
Acked-by: Raphael Norwitz
On Thu, Apr 22, 2021 at 07:02:19PM +0200, Kevin Wolf wrote:
> VIRTIO_F_RING_PACKED and VIRTIO_F_IOMMU_PLATFORM need to be supported by
> the vhost device, otherwise advertising it to the guest doesn't result
> in a working configuration. They are currently not supported by the
> vhost-user-blk export in QEMU.
>
> Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1935020
> Signed-off-by: Kevin Wolf
> ---
> hw/block/vhost-user-blk.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 8422a29470..b6f4bb3f6f 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -47,6 +47,8 @@ static const int user_feature_bits[] = {
> VIRTIO_RING_F_INDIRECT_DESC,
> VIRTIO_RING_F_EVENT_IDX,
> VIRTIO_F_NOTIFY_ON_EMPTY,
> +VIRTIO_F_RING_PACKED,
> +VIRTIO_F_IOMMU_PLATFORM,
> VHOST_INVALID_FEATURE_BIT
> };
>
> --
> 2.30.2
>
Re: [PATCH] hw/ide: Fix crash when plugging a piix3-ide device into the x-remote machine
On Wed, Apr 28, 2021 at 04:18:17PM +0200, Markus Armbruster wrote: > Stefan Hajnoczi writes: > > > On Tue, Apr 27, 2021 at 02:02:27PM -0400, John Snow wrote: > >> On 4/27/21 1:54 PM, Philippe Mathieu-Daudé wrote: > >> > On 4/27/21 7:16 PM, John Snow wrote: > >> > > On 4/27/21 9:54 AM, Stefan Hajnoczi wrote: > >> > > > I suggest fixing this at the qdev level. Make piix3-ide have a > >> > > > sub-device that inherits from ISA_DEVICE so it can only be > >> > > > instantiated > >> > > > when there's an ISA bus. > >> > > > > >> > > > Stefan > >> > > > >> > > My qdev knowledge is shaky. Does this imply that you agree with the > >> > > direction of Thomas's patch, or do you just mean to disagree with Phil > >> > > on his preferred course of action? > >> > > >> > My understanding is a disagreement to both, with a 3rd direction :) > >> > > >> > I agree with Stefan direction but I'm not sure (yet) that a sub-device > >> > is the best (long-term) solution. I guess there is a design issue with > >> > this device, and would like to understanding it first. > >> > > >> > IIUC Stefan says the piix3-ide is both a PCI and IDE device, but QOM > >> > only allow a single parent. Multiple QOM inheritance is resolved as > >> > interfaces, but PCI/IDE qdev aren't interfaces, rather abstract objects. > >> > So he suggests to embed an IDE device within the PCI piix3-ide device. > >> > > >> > My view is the PIIX is a chipset that share stuffs between components, > >> > and the IDE bus belongs to the chipset PCI root (or eventually the > >> > PCI-ISA bridge, function #0). The IDE function would use the IDE bus > >> > from its root parent as a linked property. > >> > My problem is currently this device is user-creatable as a Frankenstein > >> > single PCI function, out of its chipset. I'm not sure yet this is a > >> > dead end or I could work something out. > >> > > >> > Regards, > >> > > >> > Phil. > >> > > >> > >> It sounds complicated. In the meantime, I think I am favor of taking > >> Thomas's patch because it merely adds some error routing to allow us to > >> avoid a crash. The core organizational issues of the IDE device(s) will > >> remain and can be solved later as needed. > > > > The approach in this patch is okay but we should keep in mind it only > > solves piix3-ide. ISA provides a non-qdev backdoor API and there may be > > more instances of this type of bug. > > > > A qdev fix would address the root cause and make it possible to drop the > > backdoor API, but that's probably too much work for little benefit. > > What do you mean by backdoor API? Global @isabus? Yes. It's also strange that isa_get_irq(ISADevice *dev, unsigned isairq) accepts dev = NULL as a valid argument. Stefan signature.asc Description: PGP signature
Re: [RFC] AVR watchdog
Le 4/28/21 à 8:17 PM, Michael Rolnik a écrit : Hi Fred. How can I reproduce it? Thank you. Michael Rolnik Hi Michael, First sorry for the patchew noise, I didn't meant to sent a patch just an inlined diff. For the reproducer, that's pretty straight-forward with v6.0.0-rc5: $ cat > foo.S << EOF > __start: > wdr > EOF $ avr-gcc -nostdlib -nostartfiles -mmcu=avr6 foo.S -o foo.elf $ xxx/qemu-system-avr -serial mon:stdio -nographic -no-reboot -M mega \ -bios foo.elf -d in_asm --singlestep IN: 0x: WDR Segmentation fault (core dumped) Note that I put "--singlestep" here to avoid translating NOPs after the WDR, it breaks without as well. Fred
Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
On Wed, 28 Apr 2021 at 19:02, Philippe Mathieu-Daudé wrote: > On 4/28/21 6:34 PM, Richard Henderson wrote: > > I think the test should have been > > > > if (have_system or have_tools) and > > Yes but virtiofsd is not a tool... It is a standalone binary. This is not a distinction that our build/configure system currently makes. We have three categories: * user-mode emulator binaries * system-mode emulator binaries * tools where essentially a "tool" is "any binary we build that isn't a QEMU binary proper". If virtiofsd is genuinely not related to QEMU at all, what is it doing in our source tree ? thanks -- PMM
Re: [PATCH 1/5] vhost-user-blk: Don't reconnect during initialisation
On Wed, Apr 28, 2021 at 07:31:13PM +0200, Kevin Wolf wrote:
> Am 28.04.2021 um 18:52 hat Raphael Norwitz geschrieben:
> > Given what you've shown with the use-after-free, I agree the changes
> > need to be reverted. I'm a little uneasy about removing the reconnect
> > logic from the device realization completely though.
> >
> > On Thu, Apr 22, 2021 at 07:02:17PM +0200, Kevin Wolf wrote:
> > > This is a partial revert of commits 77542d43149 and bc79c87bcde.
> > >
> > > Usually, an error during initialisation means that the configuration was
> > > wrong. Reconnecting won't make the error go away, but just turn the
> > > error condition into an endless loop. Avoid this and return errors
> > > again.
> > >
> >
> > Is that nessesarily true? As I understand it the main usecases for
> > device reconnect are to allow a device backend to be restarted after a
> > failure or to allow the backend to be upgraded without restarting the
> > guest. I agree - misconfiguration could be a common cause of a device
> > backend crashing at realize time, but couldn't there be others? Maybe
> > transient memory pressure?
> >
> > Especially in the case where one process is connecting to many different
> > vhost-user-blk instances, I could imagine power-ons and incoming
> > migrations racing with backend restarts quite frequently. Should
> > these cases cause failures?
> >
> > We can still hit the infinite looping case you describe post-realize.
> > Why should we treat pre-realize differently?
>
> I think there is one main difference between realize() and later
> operation, which is that we can actually deliver an error to the user
> during realize(). When we're just starting QEMU and processing the CLI
> arguments, failure is very obvious, and in the context of QMP
> device-add, the client is actively waiting for a result, too.
>
> Later on, there is no good way to communicate an error (writes to stderr
> just end up in some logfile at best, QAPI events can be missed), and
> even if there were, we would have to do something with the guest until
> the user/management application actually reacts to the error. The latter
> is not a problem during realize() because the device wasn't even plugged
> in yet.
>
> So I think there are good reasons why it could make sense to distinguish
> initialisation and operation.
>
Fair enough. I'm happy in this case, provided we remain resiliant
against one-off daemon restarts during realize.
> > > Additionally, calling vhost_user_blk_disconnect() from the chardev event
> > > handler could result in use-after-free because none of the
> > > initialisation code expects that the device could just go away in the
> > > middle. So removing the call fixes crashes in several places.
> > >
> > > For example, using a num-queues setting that is incompatible with the
> > > backend would result in a crash like this (dereferencing dev->opaque,
> > > which is already NULL):
> > >
> > > #0 0x55d0a4bd in vhost_user_read_cb (source=0x568f4690,
> > > condition=(G_IO_IN | G_IO_HUP), opaque=0x7fffcbf0) at
> > > ../hw/virtio/vhost-user.c:313
> > > #1 0x55d950d3 in qio_channel_fd_source_dispatch
> > > (source=0x57c3f750, callback=0x55d0a478 ,
> > > user_data=0x7fffcbf0) at ../io/channel-watch.c:84
> > > #2 0x77b32a9f in g_main_context_dispatch () at
> > > /lib64/libglib-2.0.so.0
> > > #3 0x77b84a98 in g_main_context_iterate.constprop () at
> > > /lib64/libglib-2.0.so.0
> > > #4 0x77b32163 in g_main_loop_run () at /lib64/libglib-2.0.so.0
> > > #5 0x55d0a724 in vhost_user_read (dev=0x57bc62f8,
> > > msg=0x7fffcc50) at ../hw/virtio/vhost-user.c:402
> > > #6 0x55d0ee6b in vhost_user_get_config (dev=0x57bc62f8,
> > > config=0x57bc62ac "", config_len=60) at ../hw/virtio/vhost-user.c:2133
> > > #7 0x55d56d46 in vhost_dev_get_config (hdev=0x57bc62f8,
> > > config=0x57bc62ac "", config_len=60) at ../hw/virtio/vhost.c:1566
> > > #8 0x55cdd150 in vhost_user_blk_device_realize
> > > (dev=0x57bc60b0, errp=0x7fffcf90) at
> > > ../hw/block/vhost-user-blk.c:510
> > > #9 0x55d08f6d in virtio_device_realize (dev=0x57bc60b0,
> > > errp=0x7fffcff0) at ../hw/virtio/virtio.c:3660
> > >
> > > Signed-off-by: Kevin Wolf
> > > ---
> > > hw/block/vhost-user-blk.c | 54 ++-
> > > 1 file changed, 13 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > > index f5e9682703..e824b0a759 100644
> > > --- a/hw/block/vhost-user-blk.c
> > > +++ b/hw/block/vhost-user-blk.c
> > > @@ -50,6 +50,8 @@ static const int user_feature_bits[] = {
> > > VHOST_INVALID_FEATURE_BIT
> > > };
> > >
> > > +static void vhost_user_blk_event(void *opaque, QEMUChrEvent event);
> > > +
> > > static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t
> > > *config)
> > > {
> > >
Re: [RFC] AVR watchdog
Hi Fred.
How can I reproduce it?
Thank you.
Michael Rolnik
Sent from my cell phone, please ignore typos
On Wed, Apr 28, 2021, 5:17 PM Fred Konrad wrote:
> Hi,
>
> I fall on a segfault while running the wdr instruction on AVR:
>
> (gdb) bt
> #0 0xadd0b23a in gdb_get_cpu_pid (cpu=0xaf5a4af0) at
> ../gdbstub.c:718
> #1 0xadd0b2dd in gdb_get_cpu_process (cpu=0xaf5a4af0) at
> ../gdbstub.c:743
> #2 0xadd0e477 in gdb_set_stop_cpu (cpu=0xaf5a4af0) at
> ../gdbstub.c:2742
> #3 0xadc99b96 in cpu_handle_guest_debug
> (cpu=0xaf5a4af0) at
> ../softmmu/cpus.c:306
> #4 0xadcc66ab in rr_cpu_thread_fn (arg=0xaf5a4af0) at
> ../accel/tcg/tcg-accel-ops-rr.c:224
> #5 0xadefaf12 in qemu_thread_start (args=0xaf5d9870) at
> ../util/qemu-thread-posix.c:521
> #6 0x7f692d940ea5 in start_thread () from /lib64/libpthread.so.0
> #7 0x7f692d6699fd in clone () from /lib64/libc.so.6
>
> Wondering if there are some plan/on-going work to implement this watchdog?
>
> ---
>
> Also meanwhile I though about a workaround like that:
>
> diff --git a/target/avr/helper.c b/target/avr/helper.c
> index 35e1019594..7944ed21f4 100644
> --- a/target/avr/helper.c
> +++ b/target/avr/helper.c
> @@ -24,6 +24,7 @@
> #include "exec/exec-all.h"
> #include "exec/address-spaces.h"
> #include "exec/helper-proto.h"
> +#include "sysemu/runstate.h"
>
> bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
> {
> @@ -191,7 +192,7 @@ void helper_wdr(CPUAVRState *env)
> CPUState *cs = env_cpu(env);
>
> /* WD is not implemented yet, placeholder */
> -cs->exception_index = EXCP_DEBUG;
> +qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> cpu_loop_exit(cs);
> }
>
> In the case the guest wants to reset the board through the watchdog, would
> that
> make sense to swap to that?
>
> Best Regards,
> Fred
>
Re: [PATCH 2/5] vhost-user-blk: Use Error more consistently
Code looks ok - question about the commit message.
Acked-by: Raphael Norwitz
On Thu, Apr 22, 2021 at 07:02:18PM +0200, Kevin Wolf wrote:
> Now that vhost_user_blk_connect() is not called from an event handler
> any more, but directly from vhost_user_blk_device_realize(), we don't
> have to resort to error_report() any more, but can use Error again.
vhost_user_blk_connect() is still called by vhost_user_blk_event() which
is registered as an event handler. I don't understand your point around
us having had to use error_report() before, but not anymore. Can you
clarify?
>
> With Error, the callers are responsible for adding context if necessary
> (such as the "-device" option the error refers to). Additional prefixes
> are redundant and better omitted.
>
> Signed-off-by: Kevin Wolf
> ---
> hw/block/vhost-user-blk.c | 22 +++---
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index e824b0a759..8422a29470 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -311,7 +311,7 @@ static void vhost_user_blk_reset(VirtIODevice *vdev)
> vhost_dev_free_inflight(s->inflight);
> }
>
> -static int vhost_user_blk_connect(DeviceState *dev)
> +static int vhost_user_blk_connect(DeviceState *dev, Error **errp)
> {
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> VHostUserBlk *s = VHOST_USER_BLK(vdev);
> @@ -331,8 +331,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
>
> ret = vhost_dev_init(&s->dev, &s->vhost_user, VHOST_BACKEND_TYPE_USER,
> 0);
> if (ret < 0) {
> -error_report("vhost-user-blk: vhost initialization failed: %s",
> - strerror(-ret));
> +error_setg_errno(errp, -ret, "vhost initialization failed");
> return ret;
> }
>
> @@ -340,8 +339,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
> if (virtio_device_started(vdev, vdev->status)) {
> ret = vhost_user_blk_start(vdev);
> if (ret < 0) {
> -error_report("vhost-user-blk: vhost start failed: %s",
> - strerror(-ret));
> +error_setg_errno(errp, -ret, "vhost start failed");
> return ret;
> }
> }
> @@ -380,10 +378,12 @@ static void vhost_user_blk_event(void *opaque,
> QEMUChrEvent event)
> DeviceState *dev = opaque;
> VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> VHostUserBlk *s = VHOST_USER_BLK(vdev);
> +Error *local_err = NULL;
>
> switch (event) {
> case CHR_EVENT_OPENED:
> -if (vhost_user_blk_connect(dev) < 0) {
> +if (vhost_user_blk_connect(dev, &local_err) < 0) {
> +error_report_err(local_err);
> qemu_chr_fe_disconnect(&s->chardev);
> return;
> }
> @@ -427,7 +427,7 @@ static void vhost_user_blk_device_realize(DeviceState
> *dev, Error **errp)
> int i, ret;
>
> if (!s->chardev.chr) {
> -error_setg(errp, "vhost-user-blk: chardev is mandatory");
> +error_setg(errp, "chardev is mandatory");
> return;
> }
>
> @@ -435,16 +435,16 @@ static void vhost_user_blk_device_realize(DeviceState
> *dev, Error **errp)
> s->num_queues = 1;
> }
> if (!s->num_queues || s->num_queues > VIRTIO_QUEUE_MAX) {
> -error_setg(errp, "vhost-user-blk: invalid number of IO queues");
> +error_setg(errp, "invalid number of IO queues");
> return;
> }
>
> if (!s->queue_size) {
> -error_setg(errp, "vhost-user-blk: queue size must be non-zero");
> +error_setg(errp, "queue size must be non-zero");
> return;
> }
> if (s->queue_size > VIRTQUEUE_MAX_SIZE) {
> -error_setg(errp, "vhost-user-blk: queue size must not exceed %d",
> +error_setg(errp, "queue size must not exceed %d",
> VIRTQUEUE_MAX_SIZE);
> return;
> }
> @@ -471,7 +471,7 @@ static void vhost_user_blk_device_realize(DeviceState
> *dev, Error **errp)
> goto virtio_err;
> }
>
> -if (vhost_user_blk_connect(dev) < 0) {
> +if (vhost_user_blk_connect(dev, errp) < 0) {
> qemu_chr_fe_disconnect(&s->chardev);
> goto virtio_err;
> }
> --
> 2.30.2
>
Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
On 4/28/21 6:34 PM, Richard Henderson wrote:
> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>> seccomp = not_found
>> -if not get_option('seccomp').auto() or have_system or have_tools
>> +if not get_option('seccomp').auto() or have_system or have_tools or
>> not get_option('virtiofsd').auto()
>> seccomp = dependency('libseccomp', version: '>=2.3.0',
>> required: get_option('seccomp'),
>> method: 'pkg-config', kwargs: static_kwargs)
>
> This construct is wrong, both before and after, as I read it.
>
> not get_option(foo).auto() is true for both enabled and disabled. If
> disabled, why are we examining the dependency? If auto, if we have all
> of the dependencies we want to enable the feature -- if we don't probe
> for the dependency, how can we enable it?
>
> This error seems to be offset by the OR have_* tests, for which the
> logic also seems off.
>
> I think the test should have been
>
> if (have_system or have_tools) and
Yes but virtiofsd is not a tool... It is a standalone binary.
Maybe have_system is the culprit here:
have_system = have_system or target.endswith('-softmmu')
We should somewhere add:
have_system = have_system or something('virtiofsd')
However I wonder if we aren't going to build many objects
that are irrelevant for virtiofsd.
> (not get_option('seccomp').disabled() or
> not get_option('virtiofsd').disabled())
>
> Then we need to combine the required: argument, probably like
>
> required: get_option('seccomp').enabled() or
> get_option('virtiofsd').enabled()
>
>
> r~
>
Re: [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
On 4/28/21 6:38 PM, Richard Henderson wrote:
> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>> diff --git a/util/meson.build b/util/meson.build
>> index 510765cde46..c2eda2d1374 100644
>> --- a/util/meson.build
>> +++ b/util/meson.build
>> @@ -59,12 +59,10 @@
>> util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
>> util_ss.add(files('base64.c'))
>> util_ss.add(files('buffer.c'))
>> - util_ss.add(files('bufferiszero.c'))
>>
>> util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
>>
>> util_ss.add(files('hbitmap.c'))
>> - util_ss.add(files('hexdump.c'))
>> util_ss.add(files('iova-tree.c'))
>> - util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
>> + util_ss.add(files('qemu-sockets.c', 'uri.c'))
>> util_ss.add(files('lockcnt.c'))
>> util_ss.add(files('main-loop.c'))
>> util_ss.add(files('nvdimm-utils.c'))
>> @@ -83,3 +81,9 @@
>> if_false:
>> files('filemonitor-stub.c'))
>> util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
>> endif
>> +
>> +if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
>> + util_ss.add(files('hexdump.c'))
>> + util_ss.add(files('bufferiszero.c'))
>> + util_ss.add(files('iov.c'))
>> +endif
>
> Isn't util a static library, built once? Why are we avoiding building
> these unconditionally? Surely symbols will be included in any linked
> binaries only as needed.
Yes, in this case built once for $ configure
--target-list=i386-linux-user --disable-tools --enable-virtiofsd.
Are you suggesting to remove the 'if have_block' check? This makes build
a the files pointlessly for user-mode-only builds...
Re: [PATCH 7/7] hw/nvram: Do not build FW_CFG if not required
On 4/26/21 9:35 PM, Philippe Mathieu-Daudé wrote:
> If the Kconfig 'FW_CFG' symbol is not selected, it is pointless
> to build the fw_cfg device. Update the stubs.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> stubs/fw_cfg.c | 49 ++--
> hw/nvram/meson.build | 2 +-
> 2 files changed, 48 insertions(+), 3 deletions(-)
Answering here to Laszlo's comment from:
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg05858.html
On 4/28/21 6:44 PM, Laszlo Ersek wrote:
> I don't understand why we need to add *more code* (stubs / boilerplate)
> if our goal is (apparently) to build QEMU with *fewer* devices.
The list of callers:
hw/acpi/bios-linker-loader.c:177:return fw_cfg &&
fw_cfg_dma_enabled(fw_cfg);
hw/acpi/core.c:640:fw_cfg_add_file(fw_cfg, "etc/system-states",
g_memdup(suspend, 6), 6);
hw/acpi/ghes.c:383:fw_cfg_add_file(s, ACPI_GHES_ERRORS_FW_CFG_FILE,
hardware_error->data,
hw/acpi/ghes.c:387:fw_cfg_add_file_callback(s,
ACPI_GHES_DATA_ADDR_FW_CFG_FILE, NULL, NULL,
hw/acpi/nvdimm.c:912:fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE,
state->dsm_mem->data,
hw/acpi/vmgenid.c:128:fw_cfg_add_file(s, VMGENID_GUID_FW_CFG_FILE,
guid->data,
hw/acpi/vmgenid.c:131:fw_cfg_add_file_callback(s,
VMGENID_ADDR_FW_CFG_FILE, NULL, NULL, NULL,
hw/arm/virt-acpi-build.c:870:fw_cfg_add_file(vms->fw_cfg,
ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
hw/arm/virt.c:1531:fw_cfg_add_file(vms->fw_cfg,
"etc/smbios/smbios-tables",
hw/arm/virt.c:1533:fw_cfg_add_file(vms->fw_cfg,
"etc/smbios/smbios-anchor",
hw/core/loader.c:1017:fw_cfg_add_file(fw_cfg, fw_file_name,
data, rom->romsize);
hw/core/loader.c:1074:fw_cfg_add_file_callback(fw_cfg, fw_file_name,
hw/core/loader.c:1254:fw_cfg_set_order_override(fw_cfg, order);
hw/core/loader.c:1261:fw_cfg_reset_order_override(fw_cfg);
hw/core/loader.c:919:fw_cfg_modify_file(fw_cfg, id +
strlen("/rom@"), host, length);
hw/display/ramfb.c:131:fw_cfg_add_file_callback(fw_cfg, "etc/ramfb",
hw/hppa/machine.c:104:fw_cfg_add_file(fw_cfg,
"/etc/firmware-min-version",
hw/hppa/machine.c:108:fw_cfg_add_file(fw_cfg, "/etc/cpu/tlb_entries",
hw/hppa/machine.c:112:fw_cfg_add_file(fw_cfg, "/etc/cpu/btlb_entries",
hw/hppa/machine.c:116:fw_cfg_add_file(fw_cfg, "/etc/power-button-addr",
hw/i386/acpi-build.c:2638:fw_cfg_add_file(x86ms->fw_cfg,
ACPI_BUILD_TPMLOG_FILE,
hw/i386/acpi-build.c:2648:fw_cfg_add_file(x86ms->fw_cfg,
"etc/tpm/config",
hw/i386/acpi-build.c:2667:
fw_cfg_add_file_callback(x86ms->fw_cfg, ACPI_BUILD_RSDP_FILE,
hw/i386/fw_cfg.c:130:fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
hw/i386/fw_cfg.c:181:fw_cfg_add_file(fw_cfg,
"etc/msr_feature_control", val, sizeof(*val));
hw/i386/fw_cfg.c:85:fw_cfg_add_file(fw_cfg,
"etc/smbios/smbios-tables",
hw/i386/fw_cfg.c:87:fw_cfg_add_file(fw_cfg,
"etc/smbios/smbios-anchor",
hw/i386/microvm.c:329:fw_cfg_add_file(fw_cfg, "etc/e820", e820_table,
hw/i386/pc.c:977:fw_cfg_add_file(fw_cfg,
"etc/reserved-memory-end", val, sizeof(*val));
hw/i386/x86.c:1078:if (linuxboot_dma_enabled &&
fw_cfg_dma_enabled(fw_cfg)) {
hw/isa/lpc_ich9.c:421:fw_cfg_add_file(fw_cfg,
"etc/smi/supported-features",
hw/isa/lpc_ich9.c:428:fw_cfg_add_file_callback(fw_cfg,
"etc/smi/requested-features",
hw/isa/lpc_ich9.c:433:fw_cfg_add_file_callback(fw_cfg,
"etc/smi/features-ok",
hw/misc/pvpanic-isa.c:60:fw_cfg_add_file(fw_cfg, "etc/pvpanic-port",
pvpanic_port,
hw/misc/vmcoreinfo.c:60:fw_cfg_add_file_callback(fw_cfg,
FW_CFG_VMCOREINFO_FILENAME,
hw/ppc/mac_newworld.c:526:fw_cfg_add_file(fw_cfg,
"ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
hw/ppc/mac_oldworld.c:371:fw_cfg_add_file(fw_cfg,
"ndrv/qemu_vga.ndrv", ndrv_file, ndrv_size);
hw/vfio/igd.c:565:fw_cfg_add_file(fw_cfg_find(), "etc/igd-bdsm-size",
hw/vfio/pci-quirks.c:1201:fw_cfg_add_file(fw_cfg_find(),
"etc/igd-opregion",
softmmu/vl.c:1183:if (!fw_cfg_add_from_generator(fw_cfg, name,
gen_id, errp)) {
softmmu/vl.c:1196:fw_cfg_set_order_override(fw_cfg,
FW_CFG_ORDER_OVERRIDE_USER);
softmmu/vl.c:1197:fw_cfg_add_file(fw_cfg, name, buf, size);
softmmu/vl.c:1198:fw_cfg_reset_order_override(fw_cfg);
>From this list,
I'd like to simplify hw/acpi/bios-linker-loader.c, but later.
The remaining core components are hw/core/loader.c and softmmu/vl.c:
hw/core/loader.c:1017:fw_cfg_add_file(fw_cfg, fw_file_name,
data, rom->romsize);
hw/core/loader.c:1074:fw_cfg_add_file_callback(fw_cfg, fw_file_name,
hw/core/loader.c:1254:fw_cfg_set_order_override(fw_cfg, order);
hw/core/loader.c:1261:fw_cfg_reset_order_override(fw_cfg);
hw/core/loader.c:919:fw_cfg_modify_file(fw_cfg, id +
strlen("/rom@"), host, length);
softmmu/vl.c:1183:if (!fw_cfg_add_from_generator(fw_cfg, name,
gen_id, errp)) {
softmmu/vl.c:1196:fw_cfg_set_order_override(fw_cf
Re: [PATCH 1/5] vhost-user-blk: Don't reconnect during initialisation
Am 28.04.2021 um 18:52 hat Raphael Norwitz geschrieben:
> Given what you've shown with the use-after-free, I agree the changes
> need to be reverted. I'm a little uneasy about removing the reconnect
> logic from the device realization completely though.
>
> On Thu, Apr 22, 2021 at 07:02:17PM +0200, Kevin Wolf wrote:
> > This is a partial revert of commits 77542d43149 and bc79c87bcde.
> >
> > Usually, an error during initialisation means that the configuration was
> > wrong. Reconnecting won't make the error go away, but just turn the
> > error condition into an endless loop. Avoid this and return errors
> > again.
> >
>
> Is that nessesarily true? As I understand it the main usecases for
> device reconnect are to allow a device backend to be restarted after a
> failure or to allow the backend to be upgraded without restarting the
> guest. I agree - misconfiguration could be a common cause of a device
> backend crashing at realize time, but couldn't there be others? Maybe
> transient memory pressure?
>
> Especially in the case where one process is connecting to many different
> vhost-user-blk instances, I could imagine power-ons and incoming
> migrations racing with backend restarts quite frequently. Should
> these cases cause failures?
>
> We can still hit the infinite looping case you describe post-realize.
> Why should we treat pre-realize differently?
I think there is one main difference between realize() and later
operation, which is that we can actually deliver an error to the user
during realize(). When we're just starting QEMU and processing the CLI
arguments, failure is very obvious, and in the context of QMP
device-add, the client is actively waiting for a result, too.
Later on, there is no good way to communicate an error (writes to stderr
just end up in some logfile at best, QAPI events can be missed), and
even if there were, we would have to do something with the guest until
the user/management application actually reacts to the error. The latter
is not a problem during realize() because the device wasn't even plugged
in yet.
So I think there are good reasons why it could make sense to distinguish
initialisation and operation.
> > Additionally, calling vhost_user_blk_disconnect() from the chardev event
> > handler could result in use-after-free because none of the
> > initialisation code expects that the device could just go away in the
> > middle. So removing the call fixes crashes in several places.
> >
> > For example, using a num-queues setting that is incompatible with the
> > backend would result in a crash like this (dereferencing dev->opaque,
> > which is already NULL):
> >
> > #0 0x55d0a4bd in vhost_user_read_cb (source=0x568f4690,
> > condition=(G_IO_IN | G_IO_HUP), opaque=0x7fffcbf0) at
> > ../hw/virtio/vhost-user.c:313
> > #1 0x55d950d3 in qio_channel_fd_source_dispatch
> > (source=0x57c3f750, callback=0x55d0a478 ,
> > user_data=0x7fffcbf0) at ../io/channel-watch.c:84
> > #2 0x77b32a9f in g_main_context_dispatch () at
> > /lib64/libglib-2.0.so.0
> > #3 0x77b84a98 in g_main_context_iterate.constprop () at
> > /lib64/libglib-2.0.so.0
> > #4 0x77b32163 in g_main_loop_run () at /lib64/libglib-2.0.so.0
> > #5 0x55d0a724 in vhost_user_read (dev=0x57bc62f8,
> > msg=0x7fffcc50) at ../hw/virtio/vhost-user.c:402
> > #6 0x55d0ee6b in vhost_user_get_config (dev=0x57bc62f8,
> > config=0x57bc62ac "", config_len=60) at ../hw/virtio/vhost-user.c:2133
> > #7 0x55d56d46 in vhost_dev_get_config (hdev=0x57bc62f8,
> > config=0x57bc62ac "", config_len=60) at ../hw/virtio/vhost.c:1566
> > #8 0x55cdd150 in vhost_user_blk_device_realize
> > (dev=0x57bc60b0, errp=0x7fffcf90) at
> > ../hw/block/vhost-user-blk.c:510
> > #9 0x55d08f6d in virtio_device_realize (dev=0x57bc60b0,
> > errp=0x7fffcff0) at ../hw/virtio/virtio.c:3660
> >
> > Signed-off-by: Kevin Wolf
> > ---
> > hw/block/vhost-user-blk.c | 54 ++-
> > 1 file changed, 13 insertions(+), 41 deletions(-)
> >
> > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> > index f5e9682703..e824b0a759 100644
> > --- a/hw/block/vhost-user-blk.c
> > +++ b/hw/block/vhost-user-blk.c
> > @@ -50,6 +50,8 @@ static const int user_feature_bits[] = {
> > VHOST_INVALID_FEATURE_BIT
> > };
> >
> > +static void vhost_user_blk_event(void *opaque, QEMUChrEvent event);
> > +
> > static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t
> > *config)
> > {
> > VHostUserBlk *s = VHOST_USER_BLK(vdev);
> > @@ -362,19 +364,6 @@ static void vhost_user_blk_disconnect(DeviceState *dev)
> > vhost_dev_cleanup(&s->dev);
> > }
> >
> > -static void vhost_user_blk_event(void *opaque, QEMUChrEvent event,
> > - bool realized);
> > -
> > -static void vhost_user_blk_event_realize(void *opaque, QEM
Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected
Philippe Mathieu-Daudé writes:
> Alex, Richard, do you mind reviewing this one please?
Isn't it already merged (with my r-b tag no less ;-)
f77147cd4de8c726f89b2702f7a9d0c9711d8875
Author: Philippe Mathieu-Daudé
AuthorDate: Fri Jan 22 21:44:31 2021 +0100
Commit: Paolo Bonzini
CommitDate: Mon Feb 8 14:43:55 2021 +0100
>
> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>> From: Philippe Mathieu-Daudé
>>
>> The previous attempt (commit f77147cd4de) doesn't work as
>> expected, as we still have CONFIG_TCG=1 when using:
>>
>> configure --disable-system --disable-user
>>
>> Now than we have removed the use of CONFIG_TCG from target-dependent
>> files in tests/qtest/, we can remove the unconditional definition of
>> CONFIG_TCG in config_host.
>>
>> This avoid to build a bunch of unrequired objects when building with
>> --disable-tcg (in particular the softfloat tests):
>>
>> Before:
>>
>> $ make
>> [1/812] Generating trace-qom.h with a custom command
>> ...
>>
>> After:
>>
>> $ make
>> [1/349] Generating trace-qom.h with a custom command
>> ...
>>
>> A difference of 463 objects...
>>
>> Reported-by: Claudio Fontana
>> Suggested-by: Paolo Bonzini
>> Signed-off-by: Philippe Mathieu-Daudé
>> ---
>> v3: Include Paolo's feedback:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
>> therefore o not include Alex's R-b tag.
>>
>> Cc: Richard Henderson
>> Cc: Alex Bennée
>> Cc: Emilio G. Cota
>> ---
>> meson.build | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/meson.build b/meson.build
>> index c6f4b0cf5e8..623cbe50685 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -262,7 +262,6 @@
>> language: ['c', 'cpp', 'objc'])
>>
>>accelerators += 'CONFIG_TCG'
>> - config_host += { 'CONFIG_TCG': 'y' }
>> endif
>>
>> if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>>
--
Alex Bennée
Re: [PATCH] tests/migration: fix unix socket migration
Cleber,
Maybe you could review then queue this one?
- Wainer
On 4/20/21 10:16 PM, Hyman Huang wrote:
在 2021/3/10 0:55, Philippe Mathieu-Daudé 写道:
On 3/9/21 5:00 PM, huang...@chinatelecom.cn wrote:
From: Hyman
The test aborts and error message as the following be throwed:
"No such file or directory: '/var/tmp/qemu-migrate-{pid}.migrate",
when the unix socket migration test nearly done. The reason is
qemu removes the unix socket file after migration before
guestperf.py script do it. So pre-check if the socket file exists
when removing it to prevent the guestperf program from aborting.
Signed-off-by: Hyman
---
tests/migration/guestperf/engine.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Interesting, we have in MAINTAINERS:
Python scripts
M: Eduardo Habkost
M: Cleber Rosa
S: Odd Fixes
F: scripts/*.py
F: tests/*.py
However:
./scripts/get_maintainer.pl -f tests/migration/guestperf/engine.py
get_maintainer.pl: No maintainers found, printing recent contributors.
get_maintainer.pl: Do not blindly cc: them on patches! Use common
sense.
Ping
The following patch has fixed it
https://patchew.org/QEMU/91d5978357fb8709ef61d2030984f7142847037d.1616141556.git.huang...@chinatelecom.cn/
diff --git a/tests/migration/guestperf/engine.py
b/tests/migration/guestperf/engine.py
index 83bfc3b..86d4f21 100644
--- a/tests/migration/guestperf/engine.py
+++ b/tests/migration/guestperf/engine.py
@@ -405,7 +405,7 @@ def run(self, hardware, scenario,
result_dir=os.getcwd()):
progress_history = ret[0]
qemu_timings = ret[1]
vcpu_timings = ret[2]
- if uri[0:5] == "unix:":
+ if uri[0:5] == "unix:" and os.path.exists(uri[5:]):
os.remove(uri[5:])
if self._verbose:
print("Finished migration")
Reviewed-by: Philippe Mathieu-Daudé
[PATCH v4 29/30] hw/mips: Restrict non-virtualized machines to TCG
Only the malta and loongson3-virt machines support KVM.
Restrict the other machines to TCG:
- mipssim
- magnum
- pica61
- fuloong2e
- boston
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
hw/mips/meson.build | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/hw/mips/meson.build b/hw/mips/meson.build
index 1195716dc73..dd0101ad4d8 100644
--- a/hw/mips/meson.build
+++ b/hw/mips/meson.build
@@ -1,12 +1,15 @@
mips_ss = ss.source_set()
mips_ss.add(files('bootloader.c', 'mips_int.c'))
mips_ss.add(when: 'CONFIG_FW_CFG_MIPS', if_true: files('fw_cfg.c'))
-mips_ss.add(when: 'CONFIG_FULOONG', if_true: files('fuloong2e.c'))
mips_ss.add(when: 'CONFIG_LOONGSON3V', if_true: files('loongson3_bootp.c',
'loongson3_virt.c'))
-mips_ss.add(when: 'CONFIG_JAZZ', if_true: files('jazz.c'))
mips_ss.add(when: 'CONFIG_MALTA', if_true: files('gt64xxx_pci.c', 'malta.c'))
-mips_ss.add(when: 'CONFIG_MIPSSIM', if_true: files('mipssim.c'))
-mips_ss.add(when: 'CONFIG_MIPS_BOSTON', if_true: [files('boston.c'), fdt])
mips_ss.add(when: 'CONFIG_MIPS_CPS', if_true: files('cps.c'))
+if 'CONFIG_TCG' in config_all
+mips_ss.add(when: 'CONFIG_JAZZ', if_true: files('jazz.c'))
+mips_ss.add(when: 'CONFIG_MIPSSIM', if_true: files('mipssim.c'))
+mips_ss.add(when: 'CONFIG_FULOONG', if_true: files('fuloong2e.c'))
+mips_ss.add(when: 'CONFIG_MIPS_BOSTON', if_true: [files('boston.c'), fdt])
+endif
+
hw_arch += {'mips': mips_ss}
--
2.26.3
Re: [PATCH v7 0/7] eBPF RSS support for virtio-net.
Patchew URL: https://patchew.org/QEMU/20210428164733.56547-1-and...@daynix.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210428164733.56547-1-and...@daynix.com
Subject: [PATCH v7 0/7] eBPF RSS support for virtio-net.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20210426193520.4115528-1-phi...@redhat.com ->
patchew/20210426193520.4115528-1-phi...@redhat.com
- [tag update] patchew/20210428142431.266879-1-david.edmond...@oracle.com
-> patchew/20210428142431.266879-1-david.edmond...@oracle.com
- [tag update] patchew/20210428151804.439460-1-vsement...@virtuozzo.com
-> patchew/20210428151804.439460-1-vsement...@virtuozzo.com
* [new tag] patchew/20210428164733.56547-1-and...@daynix.com ->
patchew/20210428164733.56547-1-and...@daynix.com
Switched to a new branch 'test'
842e841 MAINTAINERS: Added eBPF maintainers information.
b931e15 docs: Added eBPF documentation.
f89d2b6 virtio-net: Added eBPF RSS to virtio-net.
15a840f3 ebpf: Added eBPF RSS loader.
44bbaca ebpf: Added eBPF RSS program.
b14810b net: Added SetSteeringEBPF method for NetClientState.
f8ae997 net/tap: Added TUNSETSTEERINGEBPF code.
=== OUTPUT BEGIN ===
1/7 Checking commit f8ae997f71c2 (net/tap: Added TUNSETSTEERINGEBPF code.)
2/7 Checking commit b14810be916e (net: Added SetSteeringEBPF method for
NetClientState.)
3/7 Checking commit 44bbaca7b895 (ebpf: Added eBPF RSS program.)
Use of uninitialized value $acpi_testexpected in string eq at
./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#22:
new file mode 100755
WARNING: line over 80 characters
#211: FILE: tools/ebpf/rss.bpf.c:157:
+ * According to
https://www.iana.org/assignments/ipv6-parameters/ipv6-parameters.xhtml
WARNING: line over 80 characters
#214: FILE: tools/ebpf/rss.bpf.c:160:
+ * Need to choose reasonable amount of maximum extensions/options we may check
to find
WARNING: line over 80 characters
#281: FILE: tools/ebpf/rss.bpf.c:227:
+*l4_offset + opt_offset + offsetof(struct
ipv6_destopt_hao, addr),
total: 0 errors, 4 warnings, 591 lines checked
Patch 3/7 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/7 Checking commit 15a840f357fd (ebpf: Added eBPF RSS loader.)
Use of uninitialized value $acpi_testexpected in string eq at
./scripts/checkpatch.pl line 1529.
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#71:
new file mode 100644
WARNING: architecture specific defines should be avoided
#353: FILE: ebpf/rss.bpf.skeleton.h:4:
+#ifndef __RSS_BPF_SKEL_H__
ERROR: code indent should never use tabs
#360: FILE: ebpf/rss.bpf.skeleton.h:11:
+^Istruct bpf_object_skeleton *skeleton;$
ERROR: code indent should never use tabs
#361: FILE: ebpf/rss.bpf.skeleton.h:12:
+^Istruct bpf_object *obj;$
ERROR: code indent should never use tabs
#362: FILE: ebpf/rss.bpf.skeleton.h:13:
+^Istruct {$
ERROR: code indent should never use tabs
#363: FILE: ebpf/rss.bpf.skeleton.h:14:
+^I^Istruct bpf_map *tap_rss_map_configurations;$
ERROR: code indent should never use tabs
#364: FILE: ebpf/rss.bpf.skeleton.h:15:
+^I^Istruct bpf_map *tap_rss_map_indirection_table;$
ERROR: code indent should never use tabs
#365: FILE: ebpf/rss.bpf.skeleton.h:16:
+^I^Istruct bpf_map *tap_rss_map_toeplitz_key;$
ERROR: code indent should never use tabs
#366: FILE: ebpf/rss.bpf.skeleton.h:17:
+^I} maps;$
ERROR: code indent should never use tabs
#367: FILE: ebpf/rss.bpf.skeleton.h:18:
+^Istruct {$
ERROR: code indent should never use tabs
#368: FILE: ebpf/rss.bpf.skeleton.h:19:
+^I^Istruct bpf_program *tun_rss_steering_prog;$
ERROR: code indent should never use tabs
#369: FILE: ebpf/rss.bpf.skeleton.h:20:
+^I} progs;$
ERROR: code indent should never use tabs
#370: FILE: ebpf/rss.bpf.skeleton.h:21:
+^Istruct {$
ERROR: code indent should never use tabs
#371: FILE: ebpf/rss.bpf.skeleton.h:22:
+^I^Istruct bpf_link *tun_rss_steering_prog;$
ERROR: code indent should never use tabs
#372: FILE: ebpf/rss.bpf.skeleton.h:23:
+^I} links;$
ERROR: code indent should never use tabs
#378: FILE: ebpf/rss.bpf.skeleton.h:29:
+^Iif (!obj)$
ERROR: braces {} are necessary for all arms of this statement
#378: FILE: ebpf/rss.bpf.skeleton.h:29:
+ if (!obj)
[...]
ERROR: code indent should never use tabs
#379: FILE: ebpf/rss.bpf.skeleton.h:30:
+^I^Ireturn;$
ERROR: code indent should never use tabs
#380: FILE: ebpf/rss.bpf.skeleton.h:31:
+^Iif (obj->skeleton)$
ERROR: braces {} are neces
Re: [PATCH v2 1/7] tests/acceptance: Automatic set -cpu to the test vm
Hi, On 4/21/21 5:16 PM, Cleber Rosa wrote: On Thu, Apr 08, 2021 at 04:52:31PM -0300, Wainer dos Santos Moschetta wrote: This introduces a new feature to the functional tests: automatic setting of the '-cpu VALUE' option to the created vm if the test is tagged with 'cpu:VALUE'. The 'cpu' property is made available to the test object as well. For example, for a simple test as: def test(self): """ :avocado: tags=cpu:host """ self.assertEqual(self.cpu, "host") self.vm.launch() So I tried a few tests with different CPU models and it works as expected. One minor caveat is that using "host" has side effects in some cases, causing tests to fail because they may also require KVM to be enabled. But this is a generic mechanism so I don't think it should be concerned with that. Good point. Certainly I will consider this when reviewing new tests. The resulting QEMU evocation will be like: qemu-system-x86_64 -display none -vga none -chardev socket,id=mon,path=/var/tmp/avo_qemu_sock_pdgzbgd_/qemu-1135557-monitor.sock -mon chardev=mon,mode=control -cpu host Only thing is: can we please just break this line (I could not ignore a 174 character line :). Signed-off-by: Wainer dos Santos Moschetta --- docs/devel/testing.rst| 17 + tests/acceptance/avocado_qemu/__init__.py | 5 + 2 files changed, 22 insertions(+) With the line broken mentioned above (which I can take care of when queueing this patch): I will send a v3 to address your review for patch 06, so I can take care of it. Reviewed-by: Cleber Rosa Tested-by: Cleber Rosa Thanks for the reviews! - Wainer
[PATCH v4 26/30] target/mips: Move exception management code to exception.c
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/internal.h | 13 ---
target/mips/tcg/tcg-internal.h | 14 +++
target/mips/cpu.c | 113 --
target/mips/exception.c| 167 +
target/mips/op_helper.c| 37
target/mips/meson.build| 1 +
6 files changed, 182 insertions(+), 163 deletions(-)
create mode 100644 target/mips/exception.c
diff --git a/target/mips/internal.h b/target/mips/internal.h
index a1c7f658c2b..07573c3e38f 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -80,7 +80,6 @@ extern const char fregnames[32][4];
extern const struct mips_def_t mips_defs[];
extern const int mips_defs_number;
-bool mips_cpu_exec_interrupt(CPUState *cpu, int int_req);
int mips_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
int mips_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
void mips_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
@@ -410,16 +409,4 @@ void sync_c0_status(CPUMIPSState *env, CPUMIPSState *cpu,
int tc);
void cpu_mips_store_status(CPUMIPSState *env, target_ulong val);
void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val);
-const char *mips_exception_name(int32_t exception);
-
-void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env, uint32_t
exception,
- int error_code, uintptr_t pc);
-
-static inline void QEMU_NORETURN do_raise_exception(CPUMIPSState *env,
-uint32_t exception,
-uintptr_t pc)
-{
-do_raise_exception_err(env, exception, 0, pc);
-}
-
#endif
diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h
index 73667b35778..75aa3ef98ed 100644
--- a/target/mips/tcg/tcg-internal.h
+++ b/target/mips/tcg/tcg-internal.h
@@ -14,11 +14,25 @@
#include "hw/core/cpu.h"
#include "cpu.h"
+void mips_cpu_synchronize_from_tb(CPUState *cs, const TranslationBlock *tb);
void mips_cpu_do_interrupt(CPUState *cpu);
+bool mips_cpu_exec_interrupt(CPUState *cpu, int int_req);
bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
MMUAccessType access_type, int mmu_idx,
bool probe, uintptr_t retaddr);
+const char *mips_exception_name(int32_t exception);
+
+void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env, uint32_t
exception,
+ int error_code, uintptr_t pc);
+
+static inline void QEMU_NORETURN do_raise_exception(CPUMIPSState *env,
+uint32_t exception,
+uintptr_t pc)
+{
+do_raise_exception_err(env, exception, 0, pc);
+}
+
#if !defined(CONFIG_USER_ONLY)
void mmu_init(CPUMIPSState *env, const mips_def_t *def);
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index a33e3b6c202..daa9a4791ee 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -218,112 +218,12 @@ static void mips_cpu_dump_state(CPUState *cs, FILE *f,
int flags)
}
}
-static const char * const excp_names[EXCP_LAST + 1] = {
-[EXCP_RESET] = "reset",
-[EXCP_SRESET] = "soft reset",
-[EXCP_DSS] = "debug single step",
-[EXCP_DINT] = "debug interrupt",
-[EXCP_NMI] = "non-maskable interrupt",
-[EXCP_MCHECK] = "machine check",
-[EXCP_EXT_INTERRUPT] = "interrupt",
-[EXCP_DFWATCH] = "deferred watchpoint",
-[EXCP_DIB] = "debug instruction breakpoint",
-[EXCP_IWATCH] = "instruction fetch watchpoint",
-[EXCP_AdEL] = "address error load",
-[EXCP_AdES] = "address error store",
-[EXCP_TLBF] = "TLB refill",
-[EXCP_IBE] = "instruction bus error",
-[EXCP_DBp] = "debug breakpoint",
-[EXCP_SYSCALL] = "syscall",
-[EXCP_BREAK] = "break",
-[EXCP_CpU] = "coprocessor unusable",
-[EXCP_RI] = "reserved instruction",
-[EXCP_OVERFLOW] = "arithmetic overflow",
-[EXCP_TRAP] = "trap",
-[EXCP_FPE] = "floating point",
-[EXCP_DDBS] = "debug data break store",
-[EXCP_DWATCH] = "data watchpoint",
-[EXCP_LTLBL] = "TLB modify",
-[EXCP_TLBL] = "TLB load",
-[EXCP_TLBS] = "TLB store",
-[EXCP_DBE] = "data bus error",
-[EXCP_DDBL] = "debug data break load",
-[EXCP_THREAD] = "thread",
-[EXCP_MDMX] = "MDMX",
-[EXCP_C2E] = "precise coprocessor 2",
-[EXCP_CACHE] = "cache error",
-[EXCP_TLBXI] = "TLB execute-inhibit",
-[EXCP_TLBRI] = "TLB read-inhibit",
-[EXCP_MSADIS] = "MSA disabled",
-[EXCP_MSAFPE] = "MSA floating point",
-};
-
-const char *mips_exception_name(int32_t exception)
-{
-if (exception < 0 || exception > EXCP_LAST) {
-return "unknown";
-}
-return excp_names[exception];
-}
-
void cpu_set_exception_base(int vp_index, target_ulong address)
{
MIPSCPU *vp = MIPS_CPU(qemu_get_cpu(vp_index));
vp->env.excep
Re: [PATCH v2] vfio-ccw: Attempt to clean up all IRQs on error
On Wed, 28 Apr 2021 16:36:52 +0200 Eric Farman wrote: > The vfio_ccw_unrealize() routine makes an unconditional attempt to > unregister every IRQ notifier, though they may not have been registered > in the first place (when running on an older kernel, for example). > > Let's mirror this behavior in the error cleanups in vfio_ccw_realize() > so that if/when new IRQs are added, it is less confusing to recognize > the necessary procedures. The worst case scenario would be some extra > messages about an undefined IRQ, but since this is an error exit that > won't be the only thing to worry about. > > And regarding those messages, let's change it to a warning instead of > an error, to better reflect their severity. The existing code in both > paths handles everything anyway. > > Signed-off-by: Eric Farman > --- > > Notes: > v1->v2: > - Downgrade unregister IRQ message from error to warning [CH] > > v1: > https://lore.kernel.org/qemu-devel/20210427142511.2125733-1-far...@linux.ibm.com/ > > hw/vfio/ccw.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) Thanks, applied.
Re: [PATCH v4 12/12] tests/meson: Only build softfloat objects if TCG is selected
On 4/28/21 7:06 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé writes:
>
>> Alex, Richard, do you mind reviewing this one please?
>
> Isn't it already merged (with my r-b tag no less ;-)
>
> f77147cd4de8c726f89b2702f7a9d0c9711d8875
See ...
> Author: Philippe Mathieu-Daudé
> AuthorDate: Fri Jan 22 21:44:31 2021 +0100
> Commit: Paolo Bonzini
> CommitDate: Mon Feb 8 14:43:55 2021 +0100
>
>>
>> On 4/15/21 6:33 PM, Philippe Mathieu-Daudé wrote:
>>> From: Philippe Mathieu-Daudé
>>>
>>> The previous attempt (commit f77147cd4de) doesn't work as
... ^ this comment :(
>>> expected, as we still have CONFIG_TCG=1 when using:
>>>
>>> configure --disable-system --disable-user
>>>
>>> Now than we have removed the use of CONFIG_TCG from target-dependent
>>> files in tests/qtest/, we can remove the unconditional definition of
>>> CONFIG_TCG in config_host.
>>>
>>> This avoid to build a bunch of unrequired objects when building with
>>> --disable-tcg (in particular the softfloat tests):
>>>
>>> Before:
>>>
>>> $ make
>>> [1/812] Generating trace-qom.h with a custom command
>>> ...
>>>
>>> After:
>>>
>>> $ make
>>> [1/349] Generating trace-qom.h with a custom command
>>> ...
>>>
>>> A difference of 463 objects...
>>>
>>> Reported-by: Claudio Fontana
>>> Suggested-by: Paolo Bonzini
>>> Signed-off-by: Philippe Mathieu-Daudé
>>> ---
>>> v3: Include Paolo's feedback:
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg793872.html
>>> therefore o not include Alex's R-b tag.
>>>
>>> Cc: Richard Henderson
>>> Cc: Alex Bennée
>>> Cc: Emilio G. Cota
>>> ---
>>> meson.build | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index c6f4b0cf5e8..623cbe50685 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -262,7 +262,6 @@
>>> language: ['c', 'cpp', 'objc'])
>>>
>>>accelerators += 'CONFIG_TCG'
>>> - config_host += { 'CONFIG_TCG': 'y' }
>>> endif
>>>
>>> if 'CONFIG_KVM' not in accelerators and get_option('kvm').enabled()
>>>
>
>
[PATCH v4 25/30] target/mips: Move TLB management helpers to tcg/sysemu/tlb_helper.c
Move TLB management helpers to tcg/sysemu/tlb_helper.c.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/helper.h| 10 -
target/mips/internal.h | 7 -
target/mips/tcg/sysemu_helper.h.inc | 9 +
target/mips/op_helper.c | 333
target/mips/tcg/sysemu/tlb_helper.c | 331 +++
5 files changed, 340 insertions(+), 350 deletions(-)
diff --git a/target/mips/helper.h b/target/mips/helper.h
index d49620f9282..ba301ae160d 100644
--- a/target/mips/helper.h
+++ b/target/mips/helper.h
@@ -202,16 +202,6 @@ FOP_PROTO(sune)
FOP_PROTO(sne)
#undef FOP_PROTO
-/* Special functions */
-#ifndef CONFIG_USER_ONLY
-DEF_HELPER_1(tlbwi, void, env)
-DEF_HELPER_1(tlbwr, void, env)
-DEF_HELPER_1(tlbp, void, env)
-DEF_HELPER_1(tlbr, void, env)
-DEF_HELPER_1(tlbinv, void, env)
-DEF_HELPER_1(tlbinvf, void, env)
-DEF_HELPER_3(ginvt, void, env, tl, i32)
-#endif /* !CONFIG_USER_ONLY */
DEF_HELPER_1(rdhwr_cpunum, tl, env)
DEF_HELPER_1(rdhwr_synci_step, tl, env)
DEF_HELPER_1(rdhwr_cc, tl, env)
diff --git a/target/mips/internal.h b/target/mips/internal.h
index c1751700731..a1c7f658c2b 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -152,13 +152,6 @@ struct CPUMIPSTLBContext {
} mmu;
};
-void r4k_helper_tlbwi(CPUMIPSState *env);
-void r4k_helper_tlbwr(CPUMIPSState *env);
-void r4k_helper_tlbp(CPUMIPSState *env);
-void r4k_helper_tlbr(CPUMIPSState *env);
-void r4k_helper_tlbinv(CPUMIPSState *env);
-void r4k_helper_tlbinvf(CPUMIPSState *env);
-
void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
vaddr addr, unsigned size,
MMUAccessType access_type,
diff --git a/target/mips/tcg/sysemu_helper.h.inc
b/target/mips/tcg/sysemu_helper.h.inc
index 1ccbf687237..4353a966f97 100644
--- a/target/mips/tcg/sysemu_helper.h.inc
+++ b/target/mips/tcg/sysemu_helper.h.inc
@@ -167,6 +167,15 @@ DEF_HELPER_1(evpe, tl, env)
DEF_HELPER_1(dvp, tl, env)
DEF_HELPER_1(evp, tl, env)
+/* TLB */
+DEF_HELPER_1(tlbwi, void, env)
+DEF_HELPER_1(tlbwr, void, env)
+DEF_HELPER_1(tlbp, void, env)
+DEF_HELPER_1(tlbr, void, env)
+DEF_HELPER_1(tlbinv, void, env)
+DEF_HELPER_1(tlbinvf, void, env)
+DEF_HELPER_3(ginvt, void, env, tl, i32)
+
/* Special */
DEF_HELPER_1(di, tl, env)
DEF_HELPER_1(ei, tl, env)
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index a7fe1de8c42..cb2a7e96fc3 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -324,339 +324,6 @@ target_ulong helper_yield(CPUMIPSState *env, target_ulong
arg)
return env->CP0_YQMask;
}
-#ifndef CONFIG_USER_ONLY
-/* TLB management */
-static void r4k_mips_tlb_flush_extra(CPUMIPSState *env, int first)
-{
-/* Discard entries from env->tlb[first] onwards. */
-while (env->tlb->tlb_in_use > first) {
-r4k_invalidate_tlb(env, --env->tlb->tlb_in_use, 0);
-}
-}
-
-static inline uint64_t get_tlb_pfn_from_entrylo(uint64_t entrylo)
-{
-#if defined(TARGET_MIPS64)
-return extract64(entrylo, 6, 54);
-#else
-return extract64(entrylo, 6, 24) | /* PFN */
- (extract64(entrylo, 32, 32) << 24); /* PFNX */
-#endif
-}
-
-static void r4k_fill_tlb(CPUMIPSState *env, int idx)
-{
-r4k_tlb_t *tlb;
-uint64_t mask = env->CP0_PageMask >> (TARGET_PAGE_BITS + 1);
-
-/* XXX: detect conflicting TLBs and raise a MCHECK exception when needed */
-tlb = &env->tlb->mmu.r4k.tlb[idx];
-if (env->CP0_EntryHi & (1 << CP0EnHi_EHINV)) {
-tlb->EHINV = 1;
-return;
-}
-tlb->EHINV = 0;
-tlb->VPN = env->CP0_EntryHi & (TARGET_PAGE_MASK << 1);
-#if defined(TARGET_MIPS64)
-tlb->VPN &= env->SEGMask;
-#endif
-tlb->ASID = env->CP0_EntryHi & env->CP0_EntryHi_ASID_mask;
-tlb->MMID = env->CP0_MemoryMapID;
-tlb->PageMask = env->CP0_PageMask;
-tlb->G = env->CP0_EntryLo0 & env->CP0_EntryLo1 & 1;
-tlb->V0 = (env->CP0_EntryLo0 & 2) != 0;
-tlb->D0 = (env->CP0_EntryLo0 & 4) != 0;
-tlb->C0 = (env->CP0_EntryLo0 >> 3) & 0x7;
-tlb->XI0 = (env->CP0_EntryLo0 >> CP0EnLo_XI) & 1;
-tlb->RI0 = (env->CP0_EntryLo0 >> CP0EnLo_RI) & 1;
-tlb->PFN[0] = (get_tlb_pfn_from_entrylo(env->CP0_EntryLo0) & ~mask) << 12;
-tlb->V1 = (env->CP0_EntryLo1 & 2) != 0;
-tlb->D1 = (env->CP0_EntryLo1 & 4) != 0;
-tlb->C1 = (env->CP0_EntryLo1 >> 3) & 0x7;
-tlb->XI1 = (env->CP0_EntryLo1 >> CP0EnLo_XI) & 1;
-tlb->RI1 = (env->CP0_EntryLo1 >> CP0EnLo_RI) & 1;
-tlb->PFN[1] = (get_tlb_pfn_from_entrylo(env->CP0_EntryLo1) & ~mask) << 12;
-}
-
-void r4k_helper_tlbinv(CPUMIPSState *env)
-{
-bool mi = !!((env->CP0_Config5 >> CP0C5_MI) & 1);
-uint16_t ASID = env->CP0_EntryHi & env->CP0_EntryHi_ASID_mask;
-uint32_t MMID = env->CP0_MemoryMapID;
-uint32_t tlb_mmid;
-r4k_tlb_t *tlb;
-int idx;
-
-MMID = mi ? MMID : (uint32_t) ASID;
-for (idx = 0; idx < env->tlb->nb_tlb
Re: [PATCH v11 2/6] arm64: kvm: Introduce MTE VM feature
On Fri, Apr 16, 2021 at 04:43:05PM +0100, Steven Price wrote:
> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 77cb2d28f2a4..5f8e165ea053 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -879,6 +879,26 @@ static int user_mem_abort(struct kvm_vcpu *vcpu,
> phys_addr_t fault_ipa,
> if (vma_pagesize == PAGE_SIZE && !force_pte)
> vma_pagesize = transparent_hugepage_adjust(memslot, hva,
> &pfn, &fault_ipa);
> +
> + if (fault_status != FSC_PERM && kvm_has_mte(kvm) && !device &&
> + pfn_valid(pfn)) {
In the current implementation, device == !pfn_valid(), so we could skip
the latter check.
> + /*
> + * VM will be able to see the page's tags, so we must ensure
> + * they have been initialised. if PG_mte_tagged is set, tags
> + * have already been initialised.
> + */
> + unsigned long i, nr_pages = vma_pagesize >> PAGE_SHIFT;
> + struct page *page = pfn_to_online_page(pfn);
> +
> + if (!page)
> + return -EFAULT;
I think that's fine, though maybe adding a comment that otherwise it
would be mapped at stage 2 as Normal Cacheable and we cannot guarantee
that the memory supports MTE tags.
> +
> + for (i = 0; i < nr_pages; i++, page++) {
> + if (!test_and_set_bit(PG_mte_tagged, &page->flags))
> + mte_clear_page_tags(page_address(page));
> + }
> + }
> +
> if (writable)
> prot |= KVM_PGTABLE_PROT_W;
I probably asked already but is the only way to map a standard RAM page
(not device) in stage 2 via the fault handler? One case I had in mind
was something like get_user_pages() but it looks like that one doesn't
call set_pte_at_notify(). There are a few other places where
set_pte_at_notify() is called and these may happen before we got a
chance to fault on stage 2, effectively populating the entry (IIUC). If
that's an issue, we could move the above loop and check closer to the
actual pte setting like kvm_pgtable_stage2_map().
While the set_pte_at() race on the page flags is somewhat clearer, we
may still have a race here with the VMM's set_pte_at() if the page is
mapped as tagged. KVM has its own mmu_lock but it wouldn't be held when
handling the VMM page tables (well, not always, see below).
gfn_to_pfn_prot() ends up calling get_user_pages*(). At least the slow
path (hva_to_pfn_slow()) ends up with FOLL_TOUCH in gup and the VMM pte
would be set, tags cleared (if PROT_MTE) before the stage 2 pte. I'm not
sure whether get_user_page_fast_only() does the same.
The race with an mprotect(PROT_MTE) in the VMM is fine I think as the
KVM mmu notifier is invoked before set_pte_at() and racing with another
user_mem_abort() is serialised by the KVM mmu_lock. The subsequent
set_pte_at() would see the PG_mte_tagged set either by the current CPU
or by the one it was racing with.
--
Catalin
Re: [PATCH v4 00/30] target/mips: Re-org to allow KVM-only builds
Patchew URL: https://patchew.org/QEMU/20210428170410.479308-1-f4...@amsat.org/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20210428170410.479308-1-f4...@amsat.org Subject: [PATCH v4 00/30] target/mips: Re-org to allow KVM-only builds === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu * [new tag] patchew/20210428170410.479308-1-f4...@amsat.org -> patchew/20210428170410.479308-1-f4...@amsat.org Switched to a new branch 'test' a0db0e0 gitlab-ci: Add KVM mips64el cross-build jobs 55de33a hw/mips: Restrict non-virtualized machines to TCG b6367df target/mips: Move TCG source files under tcg/ sub directory ea881c7 target/mips: Move CP0 helpers to sysemu/cp0.c ce95d25 target/mips: Move exception management code to exception.c 8cd822c target/mips: Move TLB management helpers to tcg/sysemu/tlb_helper.c 418c944 target/mips: Move helper_cache() to tcg/sysemu/special_helper.c b764a38 target/mips: Move Special opcodes to tcg/sysemu/special_helper.c 523789c target/mips: Restrict CPUMIPSTLBContext::map_address() handlers scope e886f40 target/mips: Move tlb_helper.c to tcg/sysemu/ aabf737 target/mips: Restrict mmu_init() to TCG 2718093 target/mips: Move sysemu TCG-specific code to tcg/sysemu/ subfolder 6d251ed target/mips: Restrict cpu_mips_get_random() / update_pagemask() to TCG 3f98aa1 target/mips: Move physical addressing code to sysemu/physaddr.c b8472f3 target/mips: Move sysemu specific files under sysemu/ subfolder 8c58af3 target/mips: Move cpu_signal_handler definition around b33009b target/mips: Add simple user-mode mips_cpu_tlb_fill() 3cbbb41 target/mips: Add simple user-mode mips_cpu_do_interrupt() d9c2b79 target/mips: Introduce tcg-internal.h for TCG specific declarations 9dbf6bd meson: Introduce meson_user_arch source set for arch-specific user-mode bce7e3c target/mips: Extract load/store helpers to ldst_helper.c e604b2d target/mips: Merge do_translate_address into cpu_mips_translate_address 458e7d6 target/mips: Declare mips_env_set_pc() inlined in "internal.h" f9b4d43 target/mips: Turn printfpr() macro into a proper function 0ae0222 target/mips: Restrict mips_cpu_dump_state() to cpu.c e3bbcaf target/mips: Optimize CPU/FPU regnames[] arrays 8d5eb80 target/mips: Make CPU/FPU regnames[] arrays global 9df82db target/mips: Move msa_reset() to new source file b37df21 target/mips: Move IEEE rounding mode array to new source file 6694ecd target/mips: Simplify meson TCG rules === OUTPUT BEGIN === 1/30 Checking commit 6694ecd2dffa (target/mips: Simplify meson TCG rules) 2/30 Checking commit b37df21d05f2 (target/mips: Move IEEE rounding mode array to new source file) Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529. WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #31: new file mode 100644 total: 0 errors, 1 warnings, 39 lines checked Patch 2/30 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 3/30 Checking commit 9df82db76e50 (target/mips: Move msa_reset() to new source file) Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529. WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #37: new file mode 100644 total: 0 errors, 1 warnings, 70 lines checked Patch 3/30 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 4/30 Checking commit 8d5eb80f7bbb (target/mips: Make CPU/FPU regnames[] arrays global) 5/30 Checking commit e3bbcaf78c8a (target/mips: Optimize CPU/FPU regnames[] arrays) 6/30 Checking commit 0ae02223ffef (target/mips: Restrict mips_cpu_dump_state() to cpu.c) 7/30 Checking commit f9b4d437bc96 (target/mips: Turn printfpr() macro into a proper function) 8/30 Checking commit 458e7d6c6a84 (target/mips: Declare mips_env_set_pc() inlined in "internal.h") 9/30 Checking commit e604b2d2a872 (target/mips: Merge do_translate_address into cpu_mips_translate_address) 10/30 Checking commit bce7e3c11816 (target/mips: Extract load/store helpers to ldst_helper.c) Use of uninitialized value $acpi_testexpected in string eq at ./scripts/checkpatch.pl line 1529. WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #18: new file mode 100644 total: 0 errors, 1 warnings, 560 lines checked Patch 10/30 has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. 11/30 Checking commit 9dbf6bd26fc0 (meson: Intro
[PATCH v4 23/30] target/mips: Move Special opcodes to tcg/sysemu/special_helper.c
Move the Special opcodes helpers to tcg/sysemu/special_helper.c.
Since mips_io_recompile_replay_branch() is set as
CPUClass::io_recompile_replay_branch handler in cpu.c,
we need to declare its prototype in "tcg-internal.h".
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/helper.h| 5 -
target/mips/tcg/tcg-internal.h | 3 +
target/mips/tcg/sysemu_helper.h.inc | 7 ++
target/mips/cpu.c | 17 ---
target/mips/op_helper.c | 100 -
target/mips/tcg/sysemu/special_helper.c | 140
target/mips/tcg/sysemu/meson.build | 1 +
7 files changed, 151 insertions(+), 122 deletions(-)
create mode 100644 target/mips/tcg/sysemu/special_helper.c
diff --git a/target/mips/helper.h b/target/mips/helper.h
index bc308e5db13..4ee7916d8b2 100644
--- a/target/mips/helper.h
+++ b/target/mips/helper.h
@@ -210,11 +210,6 @@ DEF_HELPER_1(tlbp, void, env)
DEF_HELPER_1(tlbr, void, env)
DEF_HELPER_1(tlbinv, void, env)
DEF_HELPER_1(tlbinvf, void, env)
-DEF_HELPER_1(di, tl, env)
-DEF_HELPER_1(ei, tl, env)
-DEF_HELPER_1(eret, void, env)
-DEF_HELPER_1(eretnc, void, env)
-DEF_HELPER_1(deret, void, env)
DEF_HELPER_3(ginvt, void, env, tl, i32)
#endif /* !CONFIG_USER_ONLY */
DEF_HELPER_1(rdhwr_cpunum, tl, env)
diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h
index a39ff45d58f..73667b35778 100644
--- a/target/mips/tcg/tcg-internal.h
+++ b/target/mips/tcg/tcg-internal.h
@@ -10,6 +10,7 @@
#ifndef MIPS_TCG_INTERNAL_H
#define MIPS_TCG_INTERNAL_H
+#include "tcg/tcg.h"
#include "hw/core/cpu.h"
#include "cpu.h"
@@ -27,6 +28,8 @@ void update_pagemask(CPUMIPSState *env, target_ulong arg1,
int32_t *pagemask);
void r4k_invalidate_tlb(CPUMIPSState *env, int idx, int use_extra);
uint32_t cpu_mips_get_random(CPUMIPSState *env);
+bool mips_io_recompile_replay_branch(CPUState *cs, const TranslationBlock *tb);
+
hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address,
MMUAccessType access_type, uintptr_t
retaddr);
void cpu_mips_tlb_flush(CPUMIPSState *env);
diff --git a/target/mips/tcg/sysemu_helper.h.inc
b/target/mips/tcg/sysemu_helper.h.inc
index d136c4160a7..38e55cbf118 100644
--- a/target/mips/tcg/sysemu_helper.h.inc
+++ b/target/mips/tcg/sysemu_helper.h.inc
@@ -166,3 +166,10 @@ DEF_HELPER_1(evpe, tl, env)
/* R6 Multi-threading */
DEF_HELPER_1(dvp, tl, env)
DEF_HELPER_1(evp, tl, env)
+
+/* Special */
+DEF_HELPER_1(di, tl, env)
+DEF_HELPER_1(ei, tl, env)
+DEF_HELPER_1(eret, void, env)
+DEF_HELPER_1(eretnc, void, env)
+DEF_HELPER_1(deret, void, env)
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index c3159e3d7f3..a33e3b6c202 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -342,23 +342,6 @@ static void mips_cpu_synchronize_from_tb(CPUState *cs,
env->hflags &= ~MIPS_HFLAG_BMASK;
env->hflags |= tb->flags & MIPS_HFLAG_BMASK;
}
-
-# ifndef CONFIG_USER_ONLY
-static bool mips_io_recompile_replay_branch(CPUState *cs,
-const TranslationBlock *tb)
-{
-MIPSCPU *cpu = MIPS_CPU(cs);
-CPUMIPSState *env = &cpu->env;
-
-if ((env->hflags & MIPS_HFLAG_BMASK) != 0
-&& env->active_tc.PC != tb->pc) {
-env->active_tc.PC -= (env->hflags & MIPS_HFLAG_B16 ? 2 : 4);
-env->hflags &= ~MIPS_HFLAG_BMASK;
-return true;
-}
-return false;
-}
-# endif /* !CONFIG_USER_ONLY */
#endif /* CONFIG_TCG */
static bool mips_cpu_has_work(CPUState *cs)
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 7a7369bc8a6..a077535194b 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -655,106 +655,6 @@ void helper_ginvt(CPUMIPSState *env, target_ulong arg,
uint32_t type)
}
}
-/* Specials */
-target_ulong helper_di(CPUMIPSState *env)
-{
-target_ulong t0 = env->CP0_Status;
-
-env->CP0_Status = t0 & ~(1 << CP0St_IE);
-return t0;
-}
-
-target_ulong helper_ei(CPUMIPSState *env)
-{
-target_ulong t0 = env->CP0_Status;
-
-env->CP0_Status = t0 | (1 << CP0St_IE);
-return t0;
-}
-
-static void debug_pre_eret(CPUMIPSState *env)
-{
-if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
-qemu_log("ERET: PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx,
-env->active_tc.PC, env->CP0_EPC);
-if (env->CP0_Status & (1 << CP0St_ERL)) {
-qemu_log(" ErrorEPC " TARGET_FMT_lx, env->CP0_ErrorEPC);
-}
-if (env->hflags & MIPS_HFLAG_DM) {
-qemu_log(" DEPC " TARGET_FMT_lx, env->CP0_DEPC);
-}
-qemu_log("\n");
-}
-}
-
-static void debug_post_eret(CPUMIPSState *env)
-{
-if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
-qemu_log(" => PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx,
-env->active_tc.PC, env->CP0_EPC);
-if (env->CP0_Status & (1 << CP0St_ERL)) {
-qemu_log(" ErrorEPC " T
[PATCH v4 28/30] target/mips: Move TCG source files under tcg/ sub directory
To ease maintenance, move all TCG specific files under the tcg/
sub-directory. Adapt the Meson machinery.
The following prototypes:
- mips_tcg_init()
- mips_cpu_do_unaligned_access()
- mips_cpu_do_transaction_failed()
can now be restricted to the "tcg-internal.h" header.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/helper.h | 2 +-
target/mips/internal.h | 11 ---
target/mips/tcg/tcg-internal.h | 11 +++
target/mips/{ => tcg}/msa_helper.h.inc | 0
target/mips/{ => tcg}/mips32r6.decode| 0
target/mips/{ => tcg}/mips64r6.decode| 0
target/mips/{ => tcg}/msa32.decode | 0
target/mips/{ => tcg}/msa64.decode | 0
target/mips/{ => tcg}/tx79.decode| 0
target/mips/{ => tcg}/dsp_helper.c | 0
target/mips/{ => tcg}/exception.c| 0
target/mips/{ => tcg}/fpu_helper.c | 0
target/mips/{ => tcg}/ldst_helper.c | 0
target/mips/{ => tcg}/lmmi_helper.c | 0
target/mips/{ => tcg}/msa_helper.c | 0
target/mips/{ => tcg}/msa_translate.c| 0
target/mips/{ => tcg}/mxu_translate.c| 0
target/mips/{ => tcg}/op_helper.c| 0
target/mips/{ => tcg}/rel6_translate.c | 0
target/mips/{ => tcg}/translate.c| 0
target/mips/{ => tcg}/translate_addr_const.c | 0
target/mips/{ => tcg}/tx79_translate.c | 0
target/mips/{ => tcg}/txx9_translate.c | 0
target/mips/meson.build | 31
target/mips/tcg/meson.build | 29 ++
25 files changed, 41 insertions(+), 43 deletions(-)
rename target/mips/{ => tcg}/msa_helper.h.inc (100%)
rename target/mips/{ => tcg}/mips32r6.decode (100%)
rename target/mips/{ => tcg}/mips64r6.decode (100%)
rename target/mips/{ => tcg}/msa32.decode (100%)
rename target/mips/{ => tcg}/msa64.decode (100%)
rename target/mips/{ => tcg}/tx79.decode (100%)
rename target/mips/{ => tcg}/dsp_helper.c (100%)
rename target/mips/{ => tcg}/exception.c (100%)
rename target/mips/{ => tcg}/fpu_helper.c (100%)
rename target/mips/{ => tcg}/ldst_helper.c (100%)
rename target/mips/{ => tcg}/lmmi_helper.c (100%)
rename target/mips/{ => tcg}/msa_helper.c (100%)
rename target/mips/{ => tcg}/msa_translate.c (100%)
rename target/mips/{ => tcg}/mxu_translate.c (100%)
rename target/mips/{ => tcg}/op_helper.c (100%)
rename target/mips/{ => tcg}/rel6_translate.c (100%)
rename target/mips/{ => tcg}/translate.c (100%)
rename target/mips/{ => tcg}/translate_addr_const.c (100%)
rename target/mips/{ => tcg}/tx79_translate.c (100%)
rename target/mips/{ => tcg}/txx9_translate.c (100%)
diff --git a/target/mips/helper.h b/target/mips/helper.h
index ba301ae160d..a9c6c7d1a31 100644
--- a/target/mips/helper.h
+++ b/target/mips/helper.h
@@ -608,4 +608,4 @@ DEF_HELPER_FLAGS_2(rddsp, 0, tl, tl, env)
#include "tcg/sysemu_helper.h.inc"
#endif /* !CONFIG_USER_ONLY */
-#include "msa_helper.h.inc"
+#include "tcg/msa_helper.h.inc"
diff --git a/target/mips/internal.h b/target/mips/internal.h
index dd332b4dcef..18d5da64a57 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -82,9 +82,6 @@ extern const int mips_defs_number;
int mips_cpu_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg);
int mips_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
-void mips_cpu_do_unaligned_access(CPUState *cpu, vaddr addr,
- MMUAccessType access_type,
- int mmu_idx, uintptr_t retaddr);
#define USEG_LIMIT ((target_ulong)(int32_t)0x7FFFUL)
#define KSEG0_BASE ((target_ulong)(int32_t)0x8000UL)
@@ -151,12 +148,6 @@ struct CPUMIPSTLBContext {
} mmu;
};
-void mips_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
-vaddr addr, unsigned size,
-MMUAccessType access_type,
-int mmu_idx, MemTxAttrs attrs,
-MemTxResult response, uintptr_t retaddr);
-
void sync_c0_status(CPUMIPSState *env, CPUMIPSState *cpu, int tc);
void cpu_mips_store_status(CPUMIPSState *env, target_ulong val);
void cpu_mips_store_cause(CPUMIPSState *env, target_ulong val);
@@ -209,8 +200,6 @@ static inline bool
cpu_mips_hw_interrupts_pending(CPUMIPSState *env)
return r;
}
-void mips_tcg_init(void);
-
void msa_reset(CPUMIPSState *env);
/* cp0_timer.c */
diff --git a/target/mips/tcg/tcg-internal.h b/target/mips/tcg/tcg-internal.h
index 75aa3ef98ed..81b14eb219e 100644
--- a/target/mips/tcg/tcg-internal.h
+++ b/target/mips/tcg/tcg-internal.h
@@ -11,15 +11,21 @@
#define MIPS_TCG_INTERNAL_H
#include "tcg/tcg.h"
+#include "exec/memattrs.h"
#include "hw/core/cpu.h"
#include "cpu.h"
+void mips_tcg_init(void);
+
void mips_cpu_synchronize_from_tb
Re: [PATCH 6/7] hw/{arm,hppa,riscv}: Add fw_cfg arch-specific stub
On 4/28/21 6:44 PM, Laszlo Ersek wrote: > On 04/26/21 21:35, Philippe Mathieu-Daudé wrote: >> The ARM, HPPA and RISC-V architectures don't declare any fw_cfg >> specific key. To simplify the buildsys machinery and allow building >> QEMU without the fw_cfg device (in the next commit), first add a >> per-architecture empty stub defining the fw_cfg_arch_key_name(). >> >> Update the MAINTAINERS section to cover the various target-specific >> fw_cfg.c files. >> >> Signed-off-by: Philippe Mathieu-Daudé >> --- >> hw/arm/fw_cfg.c | 19 +++ >> hw/hppa/fw_cfg.c | 19 +++ >> hw/riscv/fw_cfg.c| 19 +++ >> MAINTAINERS | 2 +- >> hw/arm/meson.build | 1 + >> hw/hppa/meson.build | 1 + >> hw/riscv/meson.build | 1 + >> 7 files changed, 61 insertions(+), 1 deletion(-) >> create mode 100644 hw/arm/fw_cfg.c >> create mode 100644 hw/hppa/fw_cfg.c >> create mode 100644 hw/riscv/fw_cfg.c > > So, I haven't commented on the Kconfig symbol wrangling yet (my comment > would be a blanket "Acked-by" anyway... sorry, not really my cup of > tea), but at this point: > > I don't understand why we need to add *more code* (stubs / boilerplate) > if our goal is (apparently) to build QEMU with *fewer* devices. > > Sorry for being dense. My total knowledge about stubs in QEMU is this: > for some QMP methods (and for some QGA methods, dependent on OS), we > need stubs. When they are invoked, they report "sorry, not implemented". > That's it: all I know about stubs. > > So... the commit message here says "simplify the buildsys", and the next > commit message says, paraphrased, "don't build fw_cfg unless we need it" > -- but why does that require more C-language code? It seems like we have > some function *calls* that shouldn't exist in an fw-cfg-less machine, in > the first place. > > Again, sorry, I'm totally dense on this. Eh no problem, I don't like this neither. If you don't mind I'll reply in the patch 7/7.
[PATCH v4 30/30] gitlab-ci: Add KVM mips64el cross-build jobs
Add a new job to cross-build the mips64el target without the TCG accelerator (IOW: only KVM accelerator enabled). Only build the mips64el target which is known to work and has users. Reviewed-by: Richard Henderson Acked-by: Thomas Huth Reviewed-by: Willian Rampazzo Signed-off-by: Philippe Mathieu-Daudé --- .gitlab-ci.d/crossbuilds.yml | 8 1 file changed, 8 insertions(+) diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml index 2d95784ed51..e44e4b49a25 100644 --- a/.gitlab-ci.d/crossbuilds.yml +++ b/.gitlab-ci.d/crossbuilds.yml @@ -176,6 +176,14 @@ cross-s390x-kvm-only: IMAGE: debian-s390x-cross ACCEL_CONFIGURE_OPTS: --disable-tcg +cross-mips64el-kvm-only: + extends: .cross_accel_build_job + needs: +job: mips64el-debian-cross-container + variables: +IMAGE: debian-mips64el-cross +ACCEL_CONFIGURE_OPTS: --disable-tcg --target-list=mips64el-softmmu + cross-win32-system: extends: .cross_system_build_job needs: -- 2.26.3
[PATCH v4 19/30] target/mips: Move sysemu TCG-specific code to tcg/sysemu/ subfolder
Move cp0_helper.c and mips-semi.c to the new tcg/sysemu/ folder,
adapting the Meson machinery.
Move the opcode definitions to tcg/sysemu_helper.h.inc.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/mips/helper.h | 166 +
target/mips/tcg/sysemu_helper.h.inc | 168 ++
target/mips/{ => tcg/sysemu}/cp0_helper.c | 0
target/mips/{ => tcg/sysemu}/mips-semi.c | 0
target/mips/meson.build | 5 -
target/mips/tcg/meson.build | 3 +
target/mips/tcg/sysemu/meson.build| 4 +
7 files changed, 179 insertions(+), 167 deletions(-)
create mode 100644 target/mips/tcg/sysemu_helper.h.inc
rename target/mips/{ => tcg/sysemu}/cp0_helper.c (100%)
rename target/mips/{ => tcg/sysemu}/mips-semi.c (100%)
create mode 100644 target/mips/tcg/sysemu/meson.build
diff --git a/target/mips/helper.h b/target/mips/helper.h
index 709494445dd..bc308e5db13 100644
--- a/target/mips/helper.h
+++ b/target/mips/helper.h
@@ -2,10 +2,6 @@ DEF_HELPER_3(raise_exception_err, noreturn, env, i32, int)
DEF_HELPER_2(raise_exception, noreturn, env, i32)
DEF_HELPER_1(raise_exception_debug, noreturn, env)
-#ifndef CONFIG_USER_ONLY
-DEF_HELPER_1(do_semihosting, void, env)
-#endif
-
#ifdef TARGET_MIPS64
DEF_HELPER_4(sdl, void, env, tl, tl, int)
DEF_HELPER_4(sdr, void, env, tl, tl, int)
@@ -42,164 +38,6 @@ DEF_HELPER_FLAGS_1(dbitswap, TCG_CALL_NO_RWG_SE, tl, tl)
DEF_HELPER_FLAGS_4(rotx, TCG_CALL_NO_RWG_SE, tl, tl, i32, i32, i32)
-#ifndef CONFIG_USER_ONLY
-/* CP0 helpers */
-DEF_HELPER_1(mfc0_mvpcontrol, tl, env)
-DEF_HELPER_1(mfc0_mvpconf0, tl, env)
-DEF_HELPER_1(mfc0_mvpconf1, tl, env)
-DEF_HELPER_1(mftc0_vpecontrol, tl, env)
-DEF_HELPER_1(mftc0_vpeconf0, tl, env)
-DEF_HELPER_1(mfc0_random, tl, env)
-DEF_HELPER_1(mfc0_tcstatus, tl, env)
-DEF_HELPER_1(mftc0_tcstatus, tl, env)
-DEF_HELPER_1(mfc0_tcbind, tl, env)
-DEF_HELPER_1(mftc0_tcbind, tl, env)
-DEF_HELPER_1(mfc0_tcrestart, tl, env)
-DEF_HELPER_1(mftc0_tcrestart, tl, env)
-DEF_HELPER_1(mfc0_tchalt, tl, env)
-DEF_HELPER_1(mftc0_tchalt, tl, env)
-DEF_HELPER_1(mfc0_tccontext, tl, env)
-DEF_HELPER_1(mftc0_tccontext, tl, env)
-DEF_HELPER_1(mfc0_tcschedule, tl, env)
-DEF_HELPER_1(mftc0_tcschedule, tl, env)
-DEF_HELPER_1(mfc0_tcschefback, tl, env)
-DEF_HELPER_1(mftc0_tcschefback, tl, env)
-DEF_HELPER_1(mfc0_count, tl, env)
-DEF_HELPER_1(mfc0_saar, tl, env)
-DEF_HELPER_1(mfhc0_saar, tl, env)
-DEF_HELPER_1(mftc0_entryhi, tl, env)
-DEF_HELPER_1(mftc0_status, tl, env)
-DEF_HELPER_1(mftc0_cause, tl, env)
-DEF_HELPER_1(mftc0_epc, tl, env)
-DEF_HELPER_1(mftc0_ebase, tl, env)
-DEF_HELPER_2(mftc0_configx, tl, env, tl)
-DEF_HELPER_1(mfc0_lladdr, tl, env)
-DEF_HELPER_1(mfc0_maar, tl, env)
-DEF_HELPER_1(mfhc0_maar, tl, env)
-DEF_HELPER_2(mfc0_watchlo, tl, env, i32)
-DEF_HELPER_2(mfc0_watchhi, tl, env, i32)
-DEF_HELPER_2(mfhc0_watchhi, tl, env, i32)
-DEF_HELPER_1(mfc0_debug, tl, env)
-DEF_HELPER_1(mftc0_debug, tl, env)
-#ifdef TARGET_MIPS64
-DEF_HELPER_1(dmfc0_tcrestart, tl, env)
-DEF_HELPER_1(dmfc0_tchalt, tl, env)
-DEF_HELPER_1(dmfc0_tccontext, tl, env)
-DEF_HELPER_1(dmfc0_tcschedule, tl, env)
-DEF_HELPER_1(dmfc0_tcschefback, tl, env)
-DEF_HELPER_1(dmfc0_lladdr, tl, env)
-DEF_HELPER_1(dmfc0_maar, tl, env)
-DEF_HELPER_2(dmfc0_watchlo, tl, env, i32)
-DEF_HELPER_2(dmfc0_watchhi, tl, env, i32)
-DEF_HELPER_1(dmfc0_saar, tl, env)
-#endif /* TARGET_MIPS64 */
-
-DEF_HELPER_2(mtc0_index, void, env, tl)
-DEF_HELPER_2(mtc0_mvpcontrol, void, env, tl)
-DEF_HELPER_2(mtc0_vpecontrol, void, env, tl)
-DEF_HELPER_2(mttc0_vpecontrol, void, env, tl)
-DEF_HELPER_2(mtc0_vpeconf0, void, env, tl)
-DEF_HELPER_2(mttc0_vpeconf0, void, env, tl)
-DEF_HELPER_2(mtc0_vpeconf1, void, env, tl)
-DEF_HELPER_2(mtc0_yqmask, void, env, tl)
-DEF_HELPER_2(mtc0_vpeopt, void, env, tl)
-DEF_HELPER_2(mtc0_entrylo0, void, env, tl)
-DEF_HELPER_2(mtc0_tcstatus, void, env, tl)
-DEF_HELPER_2(mttc0_tcstatus, void, env, tl)
-DEF_HELPER_2(mtc0_tcbind, void, env, tl)
-DEF_HELPER_2(mttc0_tcbind, void, env, tl)
-DEF_HELPER_2(mtc0_tcrestart, void, env, tl)
-DEF_HELPER_2(mttc0_tcrestart, void, env, tl)
-DEF_HELPER_2(mtc0_tchalt, void, env, tl)
-DEF_HELPER_2(mttc0_tchalt, void, env, tl)
-DEF_HELPER_2(mtc0_tccontext, void, env, tl)
-DEF_HELPER_2(mttc0_tccontext, void, env, tl)
-DEF_HELPER_2(mtc0_tcschedule, void, env, tl)
-DEF_HELPER_2(mttc0_tcschedule, void, env, tl)
-DEF_HELPER_2(mtc0_tcschefback, void, env, tl)
-DEF_HELPER_2(mttc0_tcschefback, void, env, tl)
-DEF_HELPER_2(mtc0_entrylo1, void, env, tl)
-DEF_HELPER_2(mtc0_context, void, env, tl)
-DEF_HELPER_2(mtc0_memorymapid, void, env, tl)
-DEF_HELPER_2(mtc0_pagemask, void, env, tl)
-DEF_HELPER_2(mtc0_pagegrain, void, env, tl)
-DEF_HELPER_2(mtc0_segctl0, void, env, tl)
-DEF_HELPER_2(mtc0_segctl1, void, env, tl)
-DEF_HELPER_2(mtc0_segctl2, void, env, tl)
-DEF_HELPER_2(mtc0_pwfield, void, env, tl)
-DEF_HELPER_2(mtc0_pwsize, void, env, tl)
-DEF_HELPER_2(mtc0_wired, voi
