Re: [PATCH v5 58/60] target/riscv: vector register gather instruction
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +static bool vrgather_vx_check(DisasContext *s, arg_rmrr *a)
> +{
> +return (vext_check_isa_ill(s, RVV) &&
> +vext_check_overlap_mask(s, a->rd, a->vm, true) &&
> +vext_check_reg(s, a->rd, false) &&
> +vext_check_reg(s, a->rs2, false) &&
> +(a->rd != a->rs2));
> +}
> +GEN_OPIVX_TRANS(vrgather_vx, vrgather_vx_check)
> +GEN_OPIVI_TRANS(vrgather_vi, 1, vrgather_vx, vrgather_vx_check)
The unmasked versions of these should use gvec_dup.
For the immediate version, where we can validate the index at translation time,
we can use tcg_gen_gvec_dup_mem, so that the host vector dup-from-memory
instruction can be used.
For the register version, we should re-use the code from vext.x.s where we load
the element, bound the index and squash the value to zero for index >= VLMAX.
Then use tcg_gen_gvec_dup_i64.
For the masked versions, we should load the value, as above, and then re-use
the vmerge helper with vs2 = vd, so that we get
vd[i] = v0[i].lsb ? val : vd[i]
> diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
> index 2219fdd6c5..5788e46dcf 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -4647,3 +4647,71 @@ GEN_VEXT_VSLIDE1DOWN_VX(vslide1down_vx_b, uint8_t, H1,
> clearb)
> GEN_VEXT_VSLIDE1DOWN_VX(vslide1down_vx_h, uint16_t, H2, clearh)
> GEN_VEXT_VSLIDE1DOWN_VX(vslide1down_vx_w, uint32_t, H4, clearl)
> GEN_VEXT_VSLIDE1DOWN_VX(vslide1down_vx_d, uint64_t, H8, clearq)
> +
> +/* Vector Register Gather Instruction */
> +#define GEN_VEXT_VRGATHER_VV(NAME, ETYPE, H, CLEAR_FN)\
> +void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, \
> +CPURISCVState *env, uint32_t desc)\
> +{ \
> +uint32_t mlen = vext_mlen(desc); \
> +uint32_t vlmax = env_archcpu(env)->cfg.vlen / mlen; \
> +uint32_t vm = vext_vm(desc); \
> +uint32_t vl = env->vl;\
> +uint32_t index, i;\
> + \
> +for (i = 0; i < vl; i++) {\
> +if (!vm && !vext_elem_mask(v0, mlen, i)) {\
> +continue; \
> +} \
> +index = *((ETYPE *)vs1 + H(i)); \
> +if (index >= vlmax) {
The type of index should be ETYPE or uint64_t, and similar for vlmax just so
they match.
r~
Re: [PATCH 2/8] hw/ide: Get rid of piix4_init function
On Fri, Mar 13, 2020 at 10:14:34PM +0100, BALATON Zoltan wrote:
> This removes pci_piix4_ide_init() function similar to clean up done to
> other ide devices.
>
> Signed-off-by: BALATON Zoltan
> ---
> hw/ide/piix.c| 12 +---
> hw/isa/piix4.c | 5 -
> include/hw/ide.h | 1 -
> 3 files changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ide/piix.c b/hw/ide/piix.c
> index 8bcd6b72c2..3b2de4c312 100644
> --- a/hw/ide/piix.c
> +++ b/hw/ide/piix.c
> @@ -208,17 +208,6 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
> }
> }
>
> -/* hd_table must contain 4 block drivers */
> -/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
> -PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
> -{
> -PCIDevice *dev;
> -
> -dev = pci_create_simple(bus, devfn, "piix4-ide");
> -pci_ide_create_devs(dev, hd_table);
> -return dev;
> -}
> -
> /* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
> static void piix3_ide_class_init(ObjectClass *klass, void *data)
> {
> @@ -247,6 +236,7 @@ static const TypeInfo piix3_ide_xen_info = {
> .class_init= piix3_ide_class_init,
> };
>
> +/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
> static void piix4_ide_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index 7edec5e149..0ab4787658 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -35,6 +35,7 @@
> #include "hw/timer/i8254.h"
> #include "hw/rtc/mc146818rtc.h"
> #include "hw/ide.h"
> +#include "hw/ide/pci.h"
> #include "migration/vmstate.h"
> #include "sysemu/reset.h"
> #include "sysemu/runstate.h"
> @@ -255,10 +256,12 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
> **isa_bus,
> *isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
> }
>
> +pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
> hd = g_new(DriveInfo *, ide_drives);
> ide_drive_get(hd, ide_drives);
> -pci_piix4_ide_init(pci_bus, hd, pci->devfn + 1);
> +pci_ide_create_devs(pci, hd);
> g_free(hd);
> +
Why not move pci_create_simple down, and declare a new variable?
-pci_piix4_ide_init(pci_bus, hd, pci->devfn + 1);
+pci_dev = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
+pci_ide_create_devs(pci_dev, hd);
makes it clearer what's going on imho.
> pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
> if (smbus) {
> *smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
> diff --git a/include/hw/ide.h b/include/hw/ide.h
> index 883bbaeb9b..21bd8f23f1 100644
> --- a/include/hw/ide.h
> +++ b/include/hw/ide.h
> @@ -12,7 +12,6 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int
> iobase2, int isairq,
> DriveInfo *hd0, DriveInfo *hd1);
>
> /* ide-pci.c */
> -PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
> int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
>
> /* ide-mmio.c */
> --
> 2.21.1
Re: [PATCH v5 54/60] target/riscv: integer extract instruction
On 3/14/20 10:15 PM, LIU Zhiwei wrote:
>
>
> On 2020/3/15 10:53, Richard Henderson wrote:
>> On 3/12/20 7:58 AM, LIU Zhiwei wrote:
>>> +static bool trans_vext_x_v(DisasContext *s, arg_r *a)
>>> +{
>>> + if (vext_check_isa_ill(s, RVV)) {
>>> + TCGv_ptr src2;
>>> + TCGv dest, src1;
>>> + gen_helper_vext_x_v fns[4] = {
>>> + gen_helper_vext_x_v_b, gen_helper_vext_x_v_h,
>>> + gen_helper_vext_x_v_w, gen_helper_vext_x_v_d
>>> + };
>>> +
>>> + dest = tcg_temp_new();
>>> + src1 = tcg_temp_new();
>>> + src2 = tcg_temp_new_ptr();
>>> +
>>> + gen_get_gpr(src1, a->rs1);
>>> + tcg_gen_addi_ptr(src2, cpu_env, vreg_ofs(s, a->rs2));
>>> +
>>> + fns[s->sew](dest, src2, src1, cpu_env);
>>> + gen_set_gpr(a->rd, dest);
>>> +
>>> + tcg_temp_free(dest);
>>> + tcg_temp_free(src1);
>>> + tcg_temp_free_ptr(src2);
>>> + return true;
>>> + }
>>> + return false;
>>> +}
>> This entire operation can be performed inline easily.
>>
>> static void extract_element(TCGv dest, TCGv_ptr base,
>> int ofs, int sew)
>> {
>> switch (sew) {
>> case MO_8:
>> tcg_gen_ld8u_tl(dest, base, ofs);
>> break;
>> case MO_16:
>> tcg_gen_ld16u_tl(dest, base, ofs);
>> break;
>> default:
>> tcg_gen_ld32u_tl(dest, base, ofs);
>> break;
>> #if TARGET_LONG_BITS == 64
>> case MO_64:
>> tcg_gen_ld_i64(dest, base, ofs);
>> break;
>> #endif
>> }
>> }
>>
>> static bool trans_vext_x_v(DisasContext *s, arg_r *a)
>> {
>> ...
>> if (a->rs1 == 0) {
>> /* Special case vmv.x.s rd, vs2. */
>> do_extract(dest, cpu_env,
>> vreg_ofs(s, a->rs2), s->sew);
>> } else {
>> int vlen = s->vlen >> (3 + s->sew);
>> TCGv_i32 ofs = tcg_temp_new_i32();
>> TCGv_ptr base = tcg_temp_new_ptr();
>> TCGv t_vlen, t_zero;
>>
>> /* Mask the index to the length so that we do
>> not produce an out-of-range load. */
>> tcg_gen_trunc_tl_i32(ofs, cpu_gpr[a->rs1]);
>> tcg_gen_andi_i32(ofs, ofs, vlen - 1);
>>
>> /* Convert the index to an offset. */
>> tcg_gen_shli_i32(ofs, ofs, s->sew);
>
> In big endianess host, should I convert the index first before this
> statement.
>
> #ifdef HOST_WORDS_BIGENDIAN
> static void convert_idx(TCGv_i32 idx, int sew)
> {
> switch (sew) {
> case MO_8:
> tcg_gen_xori_i32(idx, idx, 7);
> break;
> case MO_16:
> tcg_gen_xori_i32(idx, idx, 3);
> break;
> case MO_32:
> tcg_gen_xori_i32(idx, idx, 1);
> break;
> default:
> break;
> }
> }
> #endif
>
>
> When convert the index to an offset, use this function first
>
> #ifdef HOST_WORDS_BIGENDIAN
> convert_idx(ofs, s->sew)
> #endif
Yes, I forgot about endian adjust.
I would say
static void endian_adjust(TCGv_i32 ofs, int sew)
{
#ifdef HOST_WORDS_BIGENDIAN
tcg_gen_xori_i32(ofs, ofs, 7 >> sew);
#endif
}
so that you don't need the ifdef at the use site.
r~
Re: [PATCH v5 57/60] target/riscv: vector slide instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +#define GEN_VEXT_VSLIDEUP_VX(NAME, ETYPE, H, CLEAR_FN)\
> +void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> +CPURISCVState *env, uint32_t desc)\
> +{ \
> +uint32_t mlen = vext_mlen(desc); \
> +uint32_t vlmax = env_archcpu(env)->cfg.vlen / mlen; \
> +uint32_t vm = vext_vm(desc); \
> +uint32_t vl = env->vl;\
> +uint32_t offset = s1, i; \
> + \
> +if (offset > vl) {\
> +offset = vl; \
> +} \
This isn't right.
> +for (i = 0; i < vl; i++) {\
> +if (((i < offset)) || (!vm && !vext_elem_mask(v0, mlen, i))) {\
> +continue; \
> +} \
> +*((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - offset)); \
> +} \
> +if (i == 0) { \
> +return; \
> +} \
You need to eliminate vl == 0 first, not last.
Then
for (i = offset; i < vl; i++)
The types of i and vl need to be extended to target_ulong, so that you don't
incorrectly crop the input offset.
It may be worth special-casing vm=1, or hoisting it out of the loop. The
operation becomes a memcpy (at least for little-endian) at that point. See
swap_memmove in arm/sve_helper.c.
> +#define GEN_VEXT_VSLIDEDOWN_VX(NAME, ETYPE, H, CLEAR_FN) \
> +void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> +CPURISCVState *env, uint32_t desc)\
> +{ \
> +uint32_t mlen = vext_mlen(desc); \
> +uint32_t vlmax = env_archcpu(env)->cfg.vlen / mlen; \
> +uint32_t vm = vext_vm(desc); \
> +uint32_t vl = env->vl;\
> +uint32_t offset = s1, i; \
> + \
> +for (i = 0; i < vl; i++) {\
> +if (!vm && !vext_elem_mask(v0, mlen, i)) {\
> +continue; \
> +} \
> +if (i + offset < vlmax) { \
> +*((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i + offset)); \
Again, eliminate vl == 0 first. In fact, why don't we make that a global
request for all of the patches for the next revision. Checking for i == 0 last
is silly, and checks for the zero twice: once in the loop bounds and again at
the end.
It is probably worth changing the loop bounds to
if (offset >= vlmax) {
max = 0;
} else {
max = MIN(vl, vlmax - offset);
}
for (i = 0; i < max; ++i)
> +} else { \
> +*((ETYPE *)vd + H(i)) = 0;\
> +}
Which lets these zeros merge into...
> +for (; i < vlmax; i++) { \
> +CLEAR_FN(vd, vl, vl * sizeof(ETYPE), vlmax * sizeof(ETYPE)); \
> +} \
These zeros.
> +#define GEN_VEXT_VSLIDE1UP_VX(NAME, ETYPE, H, CLEAR_FN) \
> +void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void *vs2, \
> +CPURISCVState *env, uint32_t desc)\
> +{ \
> +uint32_t mlen = vext_mlen(desc); \
> +uint32_t vlmax = env_archcpu(env)->cfg.vlen / mlen; \
> +uint32_t vm = vext_vm(desc); \
> +uint32_t vl = env->vl;\
> +uint32_t i;
Re: [PATCH v5 54/60] target/riscv: integer extract instruction
On 2020/3/15 10:53, Richard Henderson wrote:
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
+static bool trans_vext_x_v(DisasContext *s, arg_r *a)
+{
+if (vext_check_isa_ill(s, RVV)) {
+TCGv_ptr src2;
+TCGv dest, src1;
+gen_helper_vext_x_v fns[4] = {
+gen_helper_vext_x_v_b, gen_helper_vext_x_v_h,
+gen_helper_vext_x_v_w, gen_helper_vext_x_v_d
+};
+
+dest = tcg_temp_new();
+src1 = tcg_temp_new();
+src2 = tcg_temp_new_ptr();
+
+gen_get_gpr(src1, a->rs1);
+tcg_gen_addi_ptr(src2, cpu_env, vreg_ofs(s, a->rs2));
+
+fns[s->sew](dest, src2, src1, cpu_env);
+gen_set_gpr(a->rd, dest);
+
+tcg_temp_free(dest);
+tcg_temp_free(src1);
+tcg_temp_free_ptr(src2);
+return true;
+}
+return false;
+}
This entire operation can be performed inline easily.
static void extract_element(TCGv dest, TCGv_ptr base,
int ofs, int sew)
{
switch (sew) {
case MO_8:
tcg_gen_ld8u_tl(dest, base, ofs);
break;
case MO_16:
tcg_gen_ld16u_tl(dest, base, ofs);
break;
default:
tcg_gen_ld32u_tl(dest, base, ofs);
break;
#if TARGET_LONG_BITS == 64
case MO_64:
tcg_gen_ld_i64(dest, base, ofs);
break;
#endif
}
}
static bool trans_vext_x_v(DisasContext *s, arg_r *a)
{
...
if (a->rs1 == 0) {
/* Special case vmv.x.s rd, vs2. */
do_extract(dest, cpu_env,
vreg_ofs(s, a->rs2), s->sew);
} else {
int vlen = s->vlen >> (3 + s->sew);
TCGv_i32 ofs = tcg_temp_new_i32();
TCGv_ptr base = tcg_temp_new_ptr();
TCGv t_vlen, t_zero;
/* Mask the index to the length so that we do
not produce an out-of-range load. */
tcg_gen_trunc_tl_i32(ofs, cpu_gpr[a->rs1]);
tcg_gen_andi_i32(ofs, ofs, vlen - 1);
/* Convert the index to an offset. */
tcg_gen_shli_i32(ofs, ofs, s->sew);
In big endianess host, should I convert the index first before this
statement.
#ifdef HOST_WORDS_BIGENDIAN
static void convert_idx(TCGv_i32 idx, int sew)
{
switch (sew) {
case MO_8:
tcg_gen_xori_i32(idx, idx, 7);
break;
case MO_16:
tcg_gen_xori_i32(idx, idx, 3);
break;
case MO_32:
tcg_gen_xori_i32(idx, idx, 1);
break;
default:
break;
}
}
#endif
When convert the index to an offset, use this function first
#ifdef HOST_WORDS_BIGENDIAN
convert_idx(ofs, s->sew)
#endif
/* Convert the index to an offset. */
tcg_gen_shli_i32(ofs, ofs, s->sew)
Zhiwei
/* Convert the index to a pointer. */
tcg_gen_extu_i32_ptr(base, ofs);
tcg_gen_add_ptr(base, base, cpu_env);
/* Perform the load. */
do_extract(dest, base,
vreg_ofs(s, a->rs2), s->sew);
tcg_temp_free_ptr(base);
tcg_temp_free_i32(ofs);
/* Flush out-of-range indexing to zero. */
t_vlen = tcg_const_tl(vlen);
t_zero = tcg_const_tl(0);
tcg_gen_movcond_tl(TCG_COND_LTU, dest, cpu_gpr[a->rs1],
t_vlen, dest, t_zero);
tcg_temp_free(t_vlen);
tcg_temp_free(t_zero);
}
r~
Re: [PATCH v5 56/60] target/riscv: floating-point scalar move instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> Signed-off-by: LIU Zhiwei
> ---
> target/riscv/helper.h | 9 +
> target/riscv/insn32.decode | 2 ++
> target/riscv/insn_trans/trans_rvv.inc.c | 47 +
> target/riscv/vector_helper.c| 36 +++
> 4 files changed, 94 insertions(+)
>
> diff --git a/target/riscv/helper.h b/target/riscv/helper.h
> index 41cecd266c..7a689a5c07 100644
> --- a/target/riscv/helper.h
> +++ b/target/riscv/helper.h
> @@ -,3 +,12 @@ DEF_HELPER_3(vmv_s_x_b, void, ptr, tl, env)
> DEF_HELPER_3(vmv_s_x_h, void, ptr, tl, env)
> DEF_HELPER_3(vmv_s_x_w, void, ptr, tl, env)
> DEF_HELPER_3(vmv_s_x_d, void, ptr, tl, env)
> +
> +DEF_HELPER_2(vfmv_f_s_b, i64, ptr, env)
> +DEF_HELPER_2(vfmv_f_s_h, i64, ptr, env)
> +DEF_HELPER_2(vfmv_f_s_w, i64, ptr, env)
> +DEF_HELPER_2(vfmv_f_s_d, i64, ptr, env)
> +DEF_HELPER_3(vfmv_s_f_b, void, ptr, i64, env)
> +DEF_HELPER_3(vfmv_s_f_h, void, ptr, i64, env)
> +DEF_HELPER_3(vfmv_s_f_w, void, ptr, i64, env)
> +DEF_HELPER_3(vfmv_s_f_d, void, ptr, i64, env)
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 7e1efeec05..bfdce0979c 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -557,6 +557,8 @@ viota_m 010110 . . 1 010 . 1010111
> @r2_vm
> vid_v 010110 . 0 10001 010 . 1010111 @r1_vm
> vext_x_v001100 1 . . 010 . 1010111 @r
> vmv_s_x 001101 1 0 . 110 . 1010111 @r2
> +vfmv_f_s001100 1 . 0 001 . 1010111 @r2rd
> +vfmv_s_f001101 1 0 . 101 . 1010111 @r2
>
> vsetvli 0 ... . 111 . 1010111 @r2_zimm
> vsetvl 100 . . 111 . 1010111 @r
> diff --git a/target/riscv/insn_trans/trans_rvv.inc.c
> b/target/riscv/insn_trans/trans_rvv.inc.c
> index 7720ffecde..99cd45b0aa 100644
> --- a/target/riscv/insn_trans/trans_rvv.inc.c
> +++ b/target/riscv/insn_trans/trans_rvv.inc.c
> @@ -2269,3 +2269,50 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x
> *a)
> }
> return false;
> }
> +
> +/* Floating-Point Scalar Move Instructions */
> +typedef void (* gen_helper_vfmv_f_s)(TCGv_i64, TCGv_ptr, TCGv_env);
> +static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
> +{
> +if (vext_check_isa_ill(s, RVV)) {
> +TCGv_ptr src2;
> +gen_helper_vfmv_f_s fns[4] = {
> +gen_helper_vfmv_f_s_b, gen_helper_vfmv_f_s_h,
> +gen_helper_vfmv_f_s_w, gen_helper_vfmv_f_s_d
> +};
> +
> +src2 = tcg_temp_new_ptr();
> +tcg_gen_addi_ptr(src2, cpu_env, vreg_ofs(s, a->rs2));
> +
> +fns[s->sew](cpu_fpr[a->rd], src2, cpu_env);
> +
> +tcg_temp_free_ptr(src2);
> +return true;
> +}
> +return false;
> +}
SEW == MO_8 should raise illegal instruction exception.
Need a check for fp enabled. Presumably
if (s->mstatus_fs == 0 || !has_ext(s, RVF)) {
return false;
}
Need to mark_fs_dirty().
Like integer vmv.x.s, this can be done inline. The nan-boxing is trivial as
well.
For 0.8, we will have to validate the nan-boxing for SEW=MO_64 && !RVD. That's
still not hard to do inline.
> +
> +typedef void (* gen_helper_vfmv_s_f)(TCGv_ptr, TCGv_i64, TCGv_env);
> +static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
> +{
> +if (vext_check_isa_ill(s, RVV | RVF) ||
> +vext_check_isa_ill(s, RVV | RVD)) {
> +TCGv_ptr dest;
> +TCGv_i64 src1;
> +gen_helper_vfmv_s_f fns[4] = {
> +gen_helper_vfmv_s_f_b, gen_helper_vfmv_s_f_h,
> +gen_helper_vfmv_s_f_w, gen_helper_vfmv_s_f_d
> +};
> +
> +src1 = tcg_temp_new_i64();
> +dest = tcg_temp_new_ptr();
> +tcg_gen_addi_ptr(dest, cpu_env, vreg_ofs(s, a->rd));
> +
> +fns[s->sew](dest, src1, cpu_env);
> +
> +tcg_temp_free_i64(src1);
> +tcg_temp_free_ptr(dest);
> +return true;
> +}
> +return false;
> +}
Again, SEW == MO_8 is illegal. Missing fp enable check.
I don't believe RVD without RVF is legal; you should not need to check for both.
Missing nan-boxing for SEW==MO_64 && FLEN==32 (!RVD). Which I think should be
done here inline, so that the uint64_t passed to the helper is always correct.
r~
Re: [PATCH v5 55/60] target/riscv: integer scalar move instruction
On 3/12/20 7:58 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 5 + > target/riscv/insn32.decode | 1 + > target/riscv/insn_trans/trans_rvv.inc.c | 26 + > target/riscv/vector_helper.c| 15 ++ > 4 files changed, 47 insertions(+) Reviewed-by: Richard Henderson What an annoying difference here between 0.7.1 and 0.8. With 0.8, we can inline this operation as for vmv.x.s. r~
Re: [PATCH v5 54/60] target/riscv: integer extract instruction
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +static bool trans_vext_x_v(DisasContext *s, arg_r *a)
> +{
> +if (vext_check_isa_ill(s, RVV)) {
> +TCGv_ptr src2;
> +TCGv dest, src1;
> +gen_helper_vext_x_v fns[4] = {
> +gen_helper_vext_x_v_b, gen_helper_vext_x_v_h,
> +gen_helper_vext_x_v_w, gen_helper_vext_x_v_d
> +};
> +
> +dest = tcg_temp_new();
> +src1 = tcg_temp_new();
> +src2 = tcg_temp_new_ptr();
> +
> +gen_get_gpr(src1, a->rs1);
> +tcg_gen_addi_ptr(src2, cpu_env, vreg_ofs(s, a->rs2));
> +
> +fns[s->sew](dest, src2, src1, cpu_env);
> +gen_set_gpr(a->rd, dest);
> +
> +tcg_temp_free(dest);
> +tcg_temp_free(src1);
> +tcg_temp_free_ptr(src2);
> +return true;
> +}
> +return false;
> +}
This entire operation can be performed inline easily.
static void extract_element(TCGv dest, TCGv_ptr base,
int ofs, int sew)
{
switch (sew) {
case MO_8:
tcg_gen_ld8u_tl(dest, base, ofs);
break;
case MO_16:
tcg_gen_ld16u_tl(dest, base, ofs);
break;
default:
tcg_gen_ld32u_tl(dest, base, ofs);
break;
#if TARGET_LONG_BITS == 64
case MO_64:
tcg_gen_ld_i64(dest, base, ofs);
break;
#endif
}
}
static bool trans_vext_x_v(DisasContext *s, arg_r *a)
{
...
if (a->rs1 == 0) {
/* Special case vmv.x.s rd, vs2. */
do_extract(dest, cpu_env,
vreg_ofs(s, a->rs2), s->sew);
} else {
int vlen = s->vlen >> (3 + s->sew);
TCGv_i32 ofs = tcg_temp_new_i32();
TCGv_ptr base = tcg_temp_new_ptr();
TCGv t_vlen, t_zero;
/* Mask the index to the length so that we do
not produce an out-of-range load. */
tcg_gen_trunc_tl_i32(ofs, cpu_gpr[a->rs1]);
tcg_gen_andi_i32(ofs, ofs, vlen - 1);
/* Convert the index to an offset. */
tcg_gen_shli_i32(ofs, ofs, s->sew);
/* Convert the index to a pointer. */
tcg_gen_extu_i32_ptr(base, ofs);
tcg_gen_add_ptr(base, base, cpu_env);
/* Perform the load. */
do_extract(dest, base,
vreg_ofs(s, a->rs2), s->sew);
tcg_temp_free_ptr(base);
tcg_temp_free_i32(ofs);
/* Flush out-of-range indexing to zero. */
t_vlen = tcg_const_tl(vlen);
t_zero = tcg_const_tl(0);
tcg_gen_movcond_tl(TCG_COND_LTU, dest, cpu_gpr[a->rs1],
t_vlen, dest, t_zero);
tcg_temp_free(t_vlen);
tcg_temp_free(t_zero);
}
r~
Re: [PATCH v5 53/60] target/riscv: vector element index instruction
On 3/12/20 7:58 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 5 + > target/riscv/insn32.decode | 2 ++ > target/riscv/insn_trans/trans_rvv.inc.c | 21 > target/riscv/vector_helper.c| 26 + > 4 files changed, 54 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v5 52/60] target/riscv: vector iota instruction
On 3/12/20 7:58 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 5 > target/riscv/insn32.decode | 1 + > target/riscv/insn_trans/trans_rvv.inc.c | 22 ++ > target/riscv/vector_helper.c| 31 + > 4 files changed, 59 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v5 50/60] target/riscv: vmfirst find-first-set mask bit
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +/* vmfirst find-first-set mask bit*/
> +target_ulong HELPER(vmfirst_m)(void *v0, void *vs2, CPURISCVState *env,
> +uint32_t desc)
> +{
> +uint32_t mlen = vext_mlen(desc);
> +uint32_t vm = vext_vm(desc);
> +uint32_t vl = env->vl;
> +int i;
> +
> +for (i = 0; i < vl; i++) {
> +if (vm || vext_elem_mask(v0, mlen, i)) {
> +if (vext_elem_mask(vs2, mlen, i)) {
> + return i;
> +}
> +}
> +}
> +return -1LL;
> +}
This is ok as-is, so
Reviewed-by: Richard Henderson
But you can do better. With the mask, as discussed, the inner loop looks like
j = mask;
j &= ((uint64_t *)vs2)[i];
j &= ((uint64_t *)v0)[i];
if (j) {
k = ctz64(j) + i * 64;
return k >> log2_mlen;
}
r~
Re: [PATCH v5 49/60] target/riscv: vector mask population count vmpopc
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +target_ulong HELPER(vmpopc_m)(void *v0, void *vs2, CPURISCVState *env,
> +uint32_t desc)
> +{
> +target_ulong cnt = 0;
> +uint32_t mlen = vext_mlen(desc);
> +uint32_t vm = vext_vm(desc);
> +uint32_t vl = env->vl;
> +int i;
> +
> +for (i = 0; i < vl; i++) {
> +if (vm || vext_elem_mask(v0, mlen, i)) {
> +if (vext_elem_mask(vs2, mlen, i)) {
> +cnt++;
> +}
> +}
> +}
> +return cnt;
> +}
This is ok as-is, so
Reviewed-by: Richard Henderson
But you can do better.
You create an array, similar to arm's pred_esz_masks[],
indexed by log2(mlen).
mask = pred_mlen_masks[log2_mlen];
n = vl >> (6 - log2_mlen);
r = extract32(vl, 0, 6 - log2_mlen);
if (r) {
rmask = extract64(mask, 0, r << log2_mlen);
} else {
rmask = 0;
}
if (vm) {
for (i = 0; i < n; i++) {
uint64_t j = ((uint64_t *)vs2)[i];
cnt += ctpop64(j & mask);
}
if (rmask) {
uint64_t j = ((uint64_t *)vs2)[i];
cnt += ctpop64(j & rmask);
}
} else {
for (i = 0; i < n; i++) {
uint64_t j = ((uint64_t *)vs2)[i];
uint64_t k = ((uint64_t *)v0)[i];
cnt += ctpop64(j & k & mask);
}
if (rmask) {
uint64_t j = ((uint64_t *)vs2)[i];
uint64_t k = ((uint64_t *)v0)[i];
cnt += ctpop64(j & k & rmask);
}
}
r~
Re: [PATCH v5 24/60] target/riscv: vector single-width averaging add and subtract
On 3/14/20 4:12 PM, LIU Zhiwei wrote:
> I am not sure whether I get it. In my opinion, the code should be modified
> like
>
> static inline int8_t aadd8_rnu(CPURISCVState *env, int8_t a, int8_t b)
> {
> int16_t res = (int16_t)a + (int16_t)b;
> uint8_t round = res & 0x1;
> res = (res >> 1) + round;
> return res;
> }
>
> static inline int8_t aadd8_rne(CPURISCVState *env, int8_t a, int8_t b)
> {
> int16_t res = (int16_t)a + (int16_t)b;
> uint8_t round = ((res & 0x3) == 0x3);
> res = (res >> 1) + round;
> return res;
> }
>
> static inline int8_t aadd8_rdn(CPURISCVState *env, int8_t a, int8_t b)
> {
> int16_t res = (int16_t)a + (int16_t)b;
> res = (res >> 1);
> return res;
> }
>
> static inline int8_t aadd8_rod(CPURISCVState *env, int8_t a, int8_t b)
> {
> int16_t res = (int16_t)a + (int16_t)b;
> uint8_t round = ((res & 0x3) == 0x1);
> res = (res >> 1) + round;
> return res;
> }
>
> RVVCALL(OPIVV2_ENV, vaadd_vv_b_rnu, OP_SSS_B, H1, H1, H1, aadd8_rnu)
> RVVCALL(OPIVV2_ENV, vaadd_vv_b_rne, OP_SSS_B, H1, H1, H1, aadd8_rne)
> RVVCALL(OPIVV2_ENV, vaadd_vv_b_rdn, OP_SSS_B, H1, H1, H1, aadd8_rdn)
> RVVCALL(OPIVV2_ENV, vaadd_vv_b_rod, OP_SSS_B, H1, H1, H1, aadd8_rod)
>
> void do_vext_vv_env(void *vd, void *v0, void *vs1,
> void *vs2, CPURISCVState *env, uint32_t desc,
> uint32_t esz, uint32_t dsz,
> opivv2_fn *fn, clear_fn *clearfn)
> {
> uint32_t vlmax = vext_maxsz(desc) / esz;
> uint32_t mlen = vext_mlen(desc);
> uint32_t vm = vext_vm(desc);
> uint32_t vl = env->vl;
> uint32_t i;
> for (i = 0; i < vl; i++) {
> if (!vm && !vext_elem_mask(v0, mlen, i)) {
> continue;
> }
> fn(vd, vs1, vs2, i, env);
> }
> if (i != 0) {
> clear_fn(vd, vl, vl * dsz, vlmax * dsz);
> }
> }
>
> #define GEN_VEXT_VV_ENV(NAME, ESZ, DSZ, CLEAR_FN) \
> void HELPER(NAME)(void *vd, void *v0, void *vs1, \
> void *vs2, CPURISCVState *env, \
> uint32_t desc) \
> { \
> static opivv2_fn *fns[4] = { \
> NAME##_rnu, NAME##_rne, \
> NAME##_rdn, NAME##_rod \
> } \
> return do_vext_vv_env(vd, v0, vs1, vs2, env, desc, \
> ESZ, DSZ, fns[env->vxrm], \
> CLEAR_FN); \
> }
>
> Is it true?
While that does look good for this case, there are many other uses of
get_round(), and it may not be quite as simple there.
My suggestion was
static inline int32_t aadd32(int vxrm, int32_t a, int32_t b)
{
int64_t res = (int64_t)a + b;
uint8_t round = get_round(vxrm, res, 1);
return (res >> 1) + round;
}
static inline int64_t aadd64(int vxrm, int64_t a, int64_t b)
{
int64_t res = a + b;
uint8_t round = get_round(vxrm, res, 1);
int64_t over = (res ^ a) & (res ^ b) & INT64_MIN;
/* With signed overflow, bit 64 is inverse of bit 63. */
return ((res >> 1) ^ over) + round;
}
RVVCALL(OPIVV2_RM, vaadd_vv_b, OP_SSS_B, H1, H1, H1, aadd32)
RVVCALL(OPIVV2_RM, vaadd_vv_h, OP_SSS_H, H2, H2, H2, aadd32)
RVVCALL(OPIVV2_RM, vaadd_vv_w, OP_SSS_W, H4, H4, H4, aadd32)
RVVCALL(OPIVV2_RM, vaadd_vv_d, OP_SSS_D, H8, H8, H8, aadd64)
static inline void
vext_vv_rm_1(void *vd, void *v0, void *vs1, void *vs2,
uint32_t vl, uint32_t vm, uint32_t mlen, int vxrm,
opivv2_rm_fn *fn)
{
for (uint32_t i = 0; i < vl; i++) {
if (!vm && !vext_elem_mask(v0, mlen, i)) {
continue;
}
fn(vd, vs1, vs2, i, vxrm);
}
}
static inline void
vext_vv_rm_2(void *vd, void *v0, void *vs1,
void *vs2, CPURISCVState *env, uint32_t desc,
uint32_t esz, uint32_t dsz,
opivv2_rm_fn *fn, clear_fn *clearfn)
{
uint32_t vlmax = vext_maxsz(desc) / esz;
uint32_t mlen = vext_mlen(desc);
uint32_t vm = vext_vm(desc);
uint32_t vl = env->vl;
if (vl == 0) {
return;
}
switch (env->vxrm) {
case 0: /* rnu */
vext_vv_rm_1(vd, v0, vs1, vs2,
vl, vm, mlen, 0, fn);
break;
case 1: /* rne */
vext_vv_rm_1(vd, v0, vs1, vs2,
vl, vm, mlen, 1, fn);
break;
case 2: /* rdn */
vext_vv_rm_1(vd, v0, vs1, vs2,
vl, vm, mlen, 2, fn);
break;
default: /* rod */
vext_vv_rm_1(vd, v0, vs1, vs2,
vl, vm, mlen, 3, fn);
break;
}
clear_fn(vd, vl, vl * dsz, vlmax * dsz);
}
>From vext_vv_rm_2, a constant is passed down all of the inline functions, so
that a constant arrives in get_round() at the bottom of the call chain. At
which point all of the expressions
Re: [PATCH v3 15/16] hw/i386/vmport: Add support for CMD_GETHZ
On 14/03/2020 23:52, Michael S. Tsirkin wrote:
On Sat, Mar 14, 2020 at 12:44:55AM +0200, Liran Alon wrote:
On 13/03/2020 22:07, Philippe Mathieu-Daudé wrote:
On 3/12/20 5:54 PM, Liran Alon wrote:
diff --git a/include/hw/i386/vmport.h b/include/hw/i386/vmport.h
index 34cc050b1ffa..aee809521aa0 100644
--- a/include/hw/i386/vmport.h
+++ b/include/hw/i386/vmport.h
@@ -12,6 +12,7 @@ typedef enum {
VMPORT_CMD_VMMOUSE_DATA = 39,
VMPORT_CMD_VMMOUSE_STATUS = 40,
VMPORT_CMD_VMMOUSE_COMMAND = 41,
+ VMPORT_CMD_GETHZ = 45,
Can you rename to something easier to read, such _GET_FREQS_HZ or nicer?
I actually prefer to stick with names similar to open-vm-tools. i.e. Similar
to the definitions in lib/include/backdoor_def.h.
Please, do not copy without attribution. It really applies everywhere,
I commented on another enum and you fixed it there, but please
go over your code and try to generally apply the same rules.
This is not a copy of the enum as the other case you replied on.
It's just names "inspired" or "similar" to original names. They are not
even the same.
This helps correlates a command in QEMU code to guest code (in
open-vm-tools) that interacts with it.
I can rename to just VMPORT_CMD_GET_HZ (Similar to what you suggested for
previous commands).
But I don't have a strong opinion on this. If you still think _GET_FREQ_HZ
is preferred, I will rename to that.
-Liran
Generally I don't think a hard to read code somewhere is a good reason
to have hard to read code in QEMU, especially since it tends to
proliferate. It seems unlikely that VMPORT_CMD_GETHZ appears verbatim
anywhere, and applying transformation rules is just too tricky. The best
way to map host code to guest code in light of coding style differences
etc is using comments. You did it in case of the type values, it
applies equally here.
Honestly, even though I used slightly different names than original
open-vm-tools code, I think it's quite trivial to coorelate.
Both by similar name (not same), by value and by function. That's why I
don't have a strong opinion about the name.
I think VMPORT_CMD_GET_HZ is sufficient, but honestly I would name it
however you want. I really don't care.
I don't think any special comment is necessary here for correlation. But
I don't mind putting above enum a general comment such as:
/* See open-vm-tools lib/include/backdoor_def.h to match these to guest
commands */
-Liran
Re: [PATCH 2/8] hw/ide: Get rid of piix4_init function
On Sat, 14 Mar 2020, Philippe Mathieu-Daudé wrote:
On 3/13/20 10:14 PM, BALATON Zoltan wrote:
This removes pci_piix4_ide_init() function similar to clean up done to
other ide devices.
Signed-off-by: BALATON Zoltan
---
hw/ide/piix.c| 12 +---
hw/isa/piix4.c | 5 -
include/hw/ide.h | 1 -
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 8bcd6b72c2..3b2de4c312 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -208,17 +208,6 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
-/* hd_table must contain 4 block drivers */
-/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
-PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int
devfn)
-{
-PCIDevice *dev;
-
-dev = pci_create_simple(bus, devfn, "piix4-ide");
-pci_ide_create_devs(dev, hd_table);
-return dev;
-}
-
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -247,6 +236,7 @@ static const TypeInfo piix3_ide_xen_info = {
.class_init= piix3_ide_class_init,
};
+/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
static void piix4_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 7edec5e149..0ab4787658 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -35,6 +35,7 @@
#include "hw/timer/i8254.h"
#include "hw/rtc/mc146818rtc.h"
#include "hw/ide.h"
+#include "hw/ide/pci.h"
#include "migration/vmstate.h"
#include "sysemu/reset.h"
#include "sysemu/runstate.h"
@@ -255,10 +256,12 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus,
*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
}
+pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
Why are you re-assigning 'pci'?
Need a place to store it to pass to pci_ide_create_devs below and pci is
unused at this point so it can be reused for this. (The variable pci
pointing to a PCIDevice was only used at the beginning of the function to
cast to dev then it's not needed any more.) Since this is very short func
and the reassign is right after its previous usage this should not be too
confusing and avoids needing to define another only once used variable fot
this. See also patch 6 (http://patchwork.ozlabs.org/patch/1254687/) that
simplifies it further.
We could also do without this variable and write:
dev = DEVICE(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(10, 0),
true, TYPE_PIIX4_PCI_DEVICE));
or after patch 6 even
pci_ide_create_devs(pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide"));
but I think those are less readable than reusing variable pci here.
Regards,
BALATON Zoltan
hd = g_new(DriveInfo *, ide_drives);
ide_drive_get(hd, ide_drives);
-pci_piix4_ide_init(pci_bus, hd, pci->devfn + 1);
+pci_ide_create_devs(pci, hd);
g_free(hd);
+
pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
if (smbus) {
*smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 883bbaeb9b..21bd8f23f1 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -12,7 +12,6 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int
iobase2, int isairq,
DriveInfo *hd0, DriveInfo *hd1);
/* ide-pci.c */
-PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int
devfn);
int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
/* ide-mmio.c */
Re: [PATCH v5 47/60] target/riscv: vector widening floating-point reduction instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 3 ++ > target/riscv/insn32.decode | 2 + > target/riscv/insn_trans/trans_rvv.inc.c | 3 ++ > target/riscv/vector_helper.c| 50 + > 4 files changed, 58 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v5 46/60] target/riscv: vector single-width floating-point reduction instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 10 +++ > target/riscv/insn32.decode | 4 +++ > target/riscv/insn_trans/trans_rvv.inc.c | 5 > target/riscv/vector_helper.c| 39 + > 4 files changed, 58 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v5 45/60] target/riscv: vector wideing integer reduction instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 7 +++ > target/riscv/insn32.decode | 2 ++ > target/riscv/insn_trans/trans_rvv.inc.c | 4 > target/riscv/vector_helper.c| 11 +++ > 4 files changed, 24 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v5 44/60] target/riscv: vector single-width integer reduction instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 33 +++ > target/riscv/insn32.decode | 8 +++ > target/riscv/insn_trans/trans_rvv.inc.c | 17 ++ > target/riscv/vector_helper.c| 76 + > 4 files changed, 134 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v5 24/60] target/riscv: vector single-width averaging add and subtract
On 2020/3/14 16:25, Richard Henderson wrote:
On 3/14/20 1:14 AM, Richard Henderson wrote:
I think you should have 4 versions of aadd8, for each of the rounding modes,
+RVVCALL(OPIVV2_ENV, vaadd_vv_b, OP_SSS_B, H1, H1, H1, aadd8)
then use this, or something like it, to define 4 functions containing main
loops, which will get the helper above inlined.
Alternately, a set of inlines, where a (constant) vxrm is passed down from
above.
I am not sure whether I get it. In my opinion, the code should be modified like
static inline int8_t aadd8_rnu(CPURISCVState *env, int8_t a, int8_t b)
{
int16_t res = (int16_t)a + (int16_t)b;
uint8_t round = res & 0x1;
res = (res >> 1) + round;
return res;
}
static inline int8_t aadd8_rne(CPURISCVState *env, int8_t a, int8_t b)
{
int16_t res = (int16_t)a + (int16_t)b;
uint8_t round = ((res & 0x3) == 0x3);
res = (res >> 1) + round;
return res;
}
static inline int8_t aadd8_rdn(CPURISCVState *env, int8_t a, int8_t b)
{
int16_t res = (int16_t)a + (int16_t)b;
res = (res >> 1);
return res;
}
static inline int8_t aadd8_rod(CPURISCVState *env, int8_t a, int8_t b)
{
int16_t res = (int16_t)a + (int16_t)b;
uint8_t round = ((res & 0x3) == 0x1);
res = (res >> 1) + round;
return res;
}
RVVCALL(OPIVV2_ENV, vaadd_vv_b_rnu, OP_SSS_B, H1, H1, H1, aadd8_rnu)
RVVCALL(OPIVV2_ENV, vaadd_vv_b_rne, OP_SSS_B, H1, H1, H1, aadd8_rne)
RVVCALL(OPIVV2_ENV, vaadd_vv_b_rdn, OP_SSS_B, H1, H1, H1, aadd8_rdn)
RVVCALL(OPIVV2_ENV, vaadd_vv_b_rod, OP_SSS_B, H1, H1, H1, aadd8_rod)
void do_vext_vv_env(void *vd, void *v0, void *vs1,
void *vs2, CPURISCVState *env, uint32_t desc,
uint32_t esz, uint32_t dsz,
opivv2_fn *fn, clear_fn *clearfn)
{
uint32_t vlmax = vext_maxsz(desc) / esz;
uint32_t mlen = vext_mlen(desc);
uint32_t vm = vext_vm(desc);
uint32_t vl = env->vl;
uint32_t i;
for (i = 0; i < vl; i++) {
if (!vm && !vext_elem_mask(v0, mlen, i)) {
continue;
}
fn(vd, vs1, vs2, i, env);
}
if (i != 0) {
clear_fn(vd, vl, vl * dsz, vlmax * dsz);
}
}
#define GEN_VEXT_VV_ENV(NAME, ESZ, DSZ, CLEAR_FN) \
void HELPER(NAME)(void *vd, void *v0, void *vs1, \
void *vs2, CPURISCVState *env, \
uint32_t desc) \
{ \
static opivv2_fn *fns[4] = { \
NAME##_rnu, NAME##_rne, \
NAME##_rdn, NAME##_rod\
} \
return do_vext_vv_env(vd, v0, vs1, vs2, env, desc,\
ESZ, DSZ, fns[env->vxrm], \
CLEAR_FN); \
}
Is it true?
Zhiwei
Then use a final outermost wrapper to select one of the 4 functions based on
env->vxrm.
The outermost wrapper could look like
switch (env->vxrm) {
case 0: somefunc(some, args, 0); break;
case 1: somefunc(some, args, 1); break;
case 2: somefunc(some, args, 2); break;
default: somefunc(some, args, 3); break;
}
so that somefunc (and its subroutines) are expanded with a constant, and we
switch on that constant at the outermost level.
r~
Re: [PATCH v5 43/60] target/riscv: narrowing floating-point/integer type-convert instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +static uint32_t vfncvtffv32(uint64_t a, float_status *s)
> +{
> +return float64_to_float32(a, s);
> +}
> +RVVCALL(OPFVV1, vfncvt_f_f_v_h, NOP_UU_H, H2, H4, vfncvtffv16)
> +RVVCALL(OPFVV1, vfncvt_f_f_v_w, NOP_UU_W, H4, H8, vfncvtffv32)
> +GEN_VEXT_V_ENV(vfncvt_f_f_v_h, 2, 2, clearh)
> +GEN_VEXT_V_ENV(vfncvt_f_f_v_w, 4, 4, clearl)
Same question as for float32_to_float64.
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH v5 42/60] target/riscv: widening floating-point/integer type-convert instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +/*
> + * vfwcvt.f.f.v vd, vs2, vm #
> + * Convert single-width float to double-width float.
> + */
> +static uint32_t vfwcvtffv16(uint16_t a, float_status *s)
> +{
> +return float16_to_float32(a, true, s);
> +}
> +static uint64_t vfwcvtffv32(uint32_t a, float_status *s)
> +{
> +return float32_to_float64(a, s);
> +}
Do you actually need this second one, as opposed to using float32_to_float64
directly?
> +RVVCALL(OPFVV1, vfwcvt_f_f_v_h, WOP_UU_H, H4, H2, vfwcvtffv16)
> +RVVCALL(OPFVV1, vfwcvt_f_f_v_w, WOP_UU_W, H8, H4, vfwcvtffv32)
> +GEN_VEXT_V_ENV(vfwcvt_f_f_v_h, 2, 4, clearl)
> +GEN_VEXT_V_ENV(vfwcvt_f_f_v_w, 4, 8, clearq)
>
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH v5 41/60] target/riscv: vector floating-point/integer type-convert instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote: > Signed-off-by: LIU Zhiwei > --- > target/riscv/helper.h | 13 ++ > target/riscv/insn32.decode | 4 +++ > target/riscv/insn_trans/trans_rvv.inc.c | 6 + > target/riscv/vector_helper.c| 33 + > 4 files changed, 56 insertions(+) Reviewed-by: Richard Henderson r~
Re: [PATCH v5 40/60] target/riscv: vector floating-point merge instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +
> +/* Vector Floating-Point Merge Instruction */
> +static bool opfvf_vfmerge_check(DisasContext *s, arg_rmrr *a)
> +{
> +return (vext_check_isa_ill(s, RVV) &&
> +vext_check_overlap_mask(s, a->rd, a->vm, false) &&
> +vext_check_reg(s, a->rd, false) &&
> +vext_check_reg(s, a->rs2, false) &&
> +((a->vm == 0) || (a->rs2 == 0)) &&
> +(s->sew != 0));
> +}
> +GEN_OPFVF_TRANS(vfmerge_vfm, opfvf_vfmerge_check)
Similar comments as for integer merge, using tcg_gen_gvec_dup_i64 for
unpredicated merges.
In fact, there's no reason at all to define a helper function for this one. I
would expect you do be able to use the exact same helpers as for the integer
merges.
r~
Re: [PATCH 4/8] hw/arm/fsl-imx31: Wire up watchdog
On 3/14/20 2:50 PM, Philippe Mathieu-Daudé wrote: [ ...] > Missing Kconfig hunk: > > -- >8 -- > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index 4cf8fa4967..8af023abde 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -366,6 +366,7 @@ config FSL_IMX31 > select SERIAL > select IMX > select IMX_I2C > + select WDT_IMX2 > select LAN9118 > > config FSL_IMX6 > --- > > With it: > Reviewed-by: Philippe Mathieu-Daudé > Done. Thanks, Guenter
Re: [PATCH 1/8] hw: Move i.MX watchdog driver to hw/watchdog
Hi Philippe, On 3/14/20 2:46 PM, Philippe Mathieu-Daudé wrote: [ ... ] > Corrected hunk: > > -- >8 -- > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index e5a876c8d1..c662d5f1e0 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -373,6 +373,7 @@ config FSL_IMX6 > select IMX > select IMX_FEC > select IMX_I2C > + select WDT_IMX2 > select SDHCI > > config ASPEED_SOC > @@ -410,6 +411,7 @@ config FSL_IMX7 > select IMX > select IMX_FEC > select IMX_I2C > + select WDT_IMX2 > select PCI_EXPRESS_DESIGNWARE > select SDHCI > select UNIMP > @@ -423,6 +425,7 @@ config FSL_IMX6UL > select IMX > select IMX_FEC > select IMX_I2C > + select WDT_IMX2 > select SDHCI > select UNIMP > Done. I also fixed >> +common-obj-$(CONFIG_WDT_IMX) += wdt_imx2.o to >> +common-obj-$(CONFIG_WDT_IMX2) += wdt_imx2.o in patch 1/8 (that had slipped to patch 2/8). Thanks a lot for the feedback, Guenter
Re: [PATCH 3/8] hw/arm/fsl-imx25: Wire up watchdog
On 3/14/20 2:48 PM, Philippe Mathieu-Daudé wrote: [ ... ] > Here you also need: > > -- >8 -- > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index c662d5f1e0..4cf8fa4967 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -358,6 +358,7 @@ config FSL_IMX25 > select IMX > select IMX_FEC > select IMX_I2C > + select WDT_IMX2 > select DS1338 > > config FSL_IMX31 > --- > > With it: > Reviewed-by: Philippe Mathieu-Daudé > Done. Thanks, Guenter
Re: [PATCH v5 39/60] target/riscv: vector floating-point classify instructions
On 3/14/20 2:15 AM, LIU Zhiwei wrote:
>>> +static uint64_t fclass_d(uint64_t frs1, float_status *s)
>>> +{
>>> + float64 f = frs1;
>>> + bool sign = float64_is_neg(f);
>>> +
>>> + if (float64_is_infinity(f)) {
>>> + return sign ? 1 << 0 : 1 << 7;
>>> + } else if (float64_is_zero(f)) {
>>> + return sign ? 1 << 3 : 1 << 4;
>>> + } else if (float64_is_zero_or_denormal(f)) {
>>> + return sign ? 1 << 2 : 1 << 5;
>>> + } else if (float64_is_any_nan(f)) {
>>> + float_status s = { }; /* for snan_bit_is_one */
>>> + return float64_is_quiet_nan(f, ) ? 1 << 9 : 1 << 8;
>>> + } else {
>>> + return sign ? 1 << 1 : 1 << 6;
>>> + }
>>> +}
>> These need to be moved out of fpu_helper.c so they can be shared.
> I will add an internals.h and move the declaration to internals.h.
Actually, let's just put declarations for them in internals.h and remove the
static. They are large enough that they don't need to be inlined.
r~
Re: [PATCH 5/8] hw/ide/pci.c: Coding style update to fix checkpatch errors
On 3/13/20 10:14 PM, BALATON Zoltan wrote:
Spaces are required around a + operator and if statements should have
braces even for single line. Also make it simpler by reversing the
condition instead of breaking the loop.
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
---
hw/ide/pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 4fc76c5225..e0c84392e2 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -485,9 +485,9 @@ void pci_ide_create_devs(PCIDevice *dev, DriveInfo
**hd_table)
int i;
for (i = 0; i < 4; i++) {
-if (hd_table[i] == NULL)
-continue;
-ide_create_drive(d->bus+bus[i], unit[i], hd_table[i]);
+if (hd_table[i]) {
+ide_create_drive(d->bus + bus[i], unit[i], hd_table[i]);
+}
}
}
Re: [PATCH 3/8] hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h
On 3/13/20 10:14 PM, BALATON Zoltan wrote: After previous patches we don't need hw/pci/pci.h any more in hw/ide.h. Some files depended on implicit inclusion by this header which are also fixed up here. Signed-off-by: BALATON Zoltan Reviewed-by: Philippe Mathieu-Daudé --- hw/ide/ahci_internal.h| 1 + include/hw/ide.h | 1 - include/hw/ide/pci.h | 1 + include/hw/misc/macio/macio.h | 1 + 4 files changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/ide/ahci_internal.h b/hw/ide/ahci_internal.h index 73424516da..bab0459774 100644 --- a/hw/ide/ahci_internal.h +++ b/hw/ide/ahci_internal.h @@ -27,6 +27,7 @@ #include "hw/ide/ahci.h" #include "hw/ide/internal.h" #include "hw/sysbus.h" +#include "hw/pci/pci.h" #define AHCI_MEM_BAR_SIZE 0x1000 #define AHCI_MAX_PORTS32 diff --git a/include/hw/ide.h b/include/hw/ide.h index 21bd8f23f1..d52c211f32 100644 --- a/include/hw/ide.h +++ b/include/hw/ide.h @@ -2,7 +2,6 @@ #define HW_IDE_H #include "hw/isa/isa.h" -#include "hw/pci/pci.h" #include "exec/memory.h" #define MAX_IDE_DEVS 2 diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h index a9f2c33e68..98ffa7dfcd 100644 --- a/include/hw/ide/pci.h +++ b/include/hw/ide/pci.h @@ -2,6 +2,7 @@ #define HW_IDE_PCI_H #include "hw/ide/internal.h" +#include "hw/pci/pci.h" #define BM_STATUS_DMAING 0x01 #define BM_STATUS_ERROR 0x02 diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h index 070a694eb5..87335a991c 100644 --- a/include/hw/misc/macio/macio.h +++ b/include/hw/misc/macio/macio.h @@ -27,6 +27,7 @@ #define MACIO_H #include "hw/char/escc.h" +#include "hw/pci/pci.h" #include "hw/ide/internal.h" #include "hw/intc/heathrow_pic.h" #include "hw/misc/macio/cuda.h"
Re: [PATCH 2/8] hw/ide: Get rid of piix4_init function
On 3/13/20 10:14 PM, BALATON Zoltan wrote:
This removes pci_piix4_ide_init() function similar to clean up done to
other ide devices.
Signed-off-by: BALATON Zoltan
---
hw/ide/piix.c| 12 +---
hw/isa/piix4.c | 5 -
include/hw/ide.h | 1 -
3 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 8bcd6b72c2..3b2de4c312 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -208,17 +208,6 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
-/* hd_table must contain 4 block drivers */
-/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
-PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
-{
-PCIDevice *dev;
-
-dev = pci_create_simple(bus, devfn, "piix4-ide");
-pci_ide_create_devs(dev, hd_table);
-return dev;
-}
-
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -247,6 +236,7 @@ static const TypeInfo piix3_ide_xen_info = {
.class_init= piix3_ide_class_init,
};
+/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
static void piix4_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index 7edec5e149..0ab4787658 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -35,6 +35,7 @@
#include "hw/timer/i8254.h"
#include "hw/rtc/mc146818rtc.h"
#include "hw/ide.h"
+#include "hw/ide/pci.h"
#include "migration/vmstate.h"
#include "sysemu/reset.h"
#include "sysemu/runstate.h"
@@ -255,10 +256,12 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
**isa_bus,
*isa_bus = ISA_BUS(qdev_get_child_bus(dev, "isa.0"));
}
+pci = pci_create_simple(pci_bus, pci->devfn + 1, "piix4-ide");
Why are you re-assigning 'pci'?
hd = g_new(DriveInfo *, ide_drives);
ide_drive_get(hd, ide_drives);
-pci_piix4_ide_init(pci_bus, hd, pci->devfn + 1);
+pci_ide_create_devs(pci, hd);
g_free(hd);
+
pci_create_simple(pci_bus, pci->devfn + 2, "piix4-usb-uhci");
if (smbus) {
*smbus = piix4_pm_init(pci_bus, pci->devfn + 3, 0x1100,
diff --git a/include/hw/ide.h b/include/hw/ide.h
index 883bbaeb9b..21bd8f23f1 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -12,7 +12,6 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2,
int isairq,
DriveInfo *hd0, DriveInfo *hd1);
/* ide-pci.c */
-PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
/* ide-mmio.c */
Re: [PATCH 1/8] hw/ide: Get rid of piix3_init functions
On 3/13/20 10:14 PM, BALATON Zoltan wrote:
This removes pci_piix3_ide_init() and pci_piix3_xen_ide_init()
functions similar to clean up done to other ide devices.
Signed-off-by: BALATON Zoltan
Reviewed-by: Philippe Mathieu-Daudé
---
hw/i386/pc_piix.c | 10 +-
hw/ide/pci.c | 1 +
hw/ide/piix.c | 21 +
include/hw/ide.h | 2 --
4 files changed, 7 insertions(+), 27 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e2d98243bc..c399398739 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -39,6 +39,7 @@
#include "hw/usb.h"
#include "net/net.h"
#include "hw/ide.h"
+#include "hw/ide/pci.h"
#include "hw/irq.h"
#include "sysemu/kvm.h"
#include "hw/kvm/clock.h"
@@ -242,11 +243,10 @@ static void pc_init1(MachineState *machine,
ide_drive_get(hd, ARRAY_SIZE(hd));
if (pcmc->pci_enabled) {
PCIDevice *dev;
-if (xen_enabled()) {
-dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
-} else {
-dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
-}
+
+dev = pci_create_simple(pci_bus, piix3_devfn + 1,
+xen_enabled() ? "piix3-ide-xen" : "piix3-ide");
+pci_ide_create_devs(dev, hd);
idebus[0] = qdev_get_child_bus(>qdev, "ide.0");
idebus[1] = qdev_get_child_bus(>qdev, "ide.1");
pc_cmos_init(pcms, idebus[0], idebus[1], rtc_state);
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 1a6a287e76..4fc76c5225 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -476,6 +476,7 @@ const VMStateDescription vmstate_ide_pci = {
}
};
+/* hd_table must contain 4 block drivers */
void pci_ide_create_devs(PCIDevice *dev, DriveInfo **hd_table)
{
PCIIDEState *d = PCI_IDE(dev);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index bc575b4d70..8bcd6b72c2 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -197,15 +197,6 @@ int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux)
return 0;
}
-PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
-{
-PCIDevice *dev;
-
-dev = pci_create_simple(bus, devfn, "piix3-ide-xen");
-pci_ide_create_devs(dev, hd_table);
-return dev;
-}
-
static void pci_piix_ide_exitfn(PCIDevice *dev)
{
PCIIDEState *d = PCI_IDE(dev);
@@ -217,17 +208,6 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
-/* hd_table must contain 4 block drivers */
-/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
-PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
-{
-PCIDevice *dev;
-
-dev = pci_create_simple(bus, devfn, "piix3-ide");
-pci_ide_create_devs(dev, hd_table);
-return dev;
-}
-
/* hd_table must contain 4 block drivers */
/* NOTE: for the PIIX4, the IRQs and IOports are hardcoded */
PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn)
@@ -239,6 +219,7 @@ PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo
**hd_table, int devfn)
return dev;
}
+/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
diff --git a/include/hw/ide.h b/include/hw/ide.h
index dea0ecf5be..883bbaeb9b 100644
--- a/include/hw/ide.h
+++ b/include/hw/ide.h
@@ -12,8 +12,6 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, int iobase2,
int isairq,
DriveInfo *hd0, DriveInfo *hd1);
/* ide-pci.c */
-PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int
devfn);
-PCIDevice *pci_piix3_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
PCIDevice *pci_piix4_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn);
int pci_piix3_xen_ide_unplug(DeviceState *dev, bool aux);
Re: [PATCH v3 15/16] hw/i386/vmport: Add support for CMD_GETHZ
On Sat, Mar 14, 2020 at 12:44:55AM +0200, Liran Alon wrote:
>
> On 13/03/2020 22:07, Philippe Mathieu-Daudé wrote:
> > On 3/12/20 5:54 PM, Liran Alon wrote:
> > >
> > > diff --git a/include/hw/i386/vmport.h b/include/hw/i386/vmport.h
> > > index 34cc050b1ffa..aee809521aa0 100644
> > > --- a/include/hw/i386/vmport.h
> > > +++ b/include/hw/i386/vmport.h
> > > @@ -12,6 +12,7 @@ typedef enum {
> > > VMPORT_CMD_VMMOUSE_DATA = 39,
> > > VMPORT_CMD_VMMOUSE_STATUS = 40,
> > > VMPORT_CMD_VMMOUSE_COMMAND = 41,
> > > + VMPORT_CMD_GETHZ = 45,
> >
> > Can you rename to something easier to read, such _GET_FREQS_HZ or nicer?
> >
> I actually prefer to stick with names similar to open-vm-tools. i.e. Similar
> to the definitions in lib/include/backdoor_def.h.
Please, do not copy without attribution. It really applies everywhere,
I commented on another enum and you fixed it there, but please
go over your code and try to generally apply the same rules.
> This helps correlates a command in QEMU code to guest code (in
> open-vm-tools) that interacts with it.
> I can rename to just VMPORT_CMD_GET_HZ (Similar to what you suggested for
> previous commands).
> But I don't have a strong opinion on this. If you still think _GET_FREQ_HZ
> is preferred, I will rename to that.
>
> -Liran
Generally I don't think a hard to read code somewhere is a good reason
to have hard to read code in QEMU, especially since it tends to
proliferate. It seems unlikely that VMPORT_CMD_GETHZ appears verbatim
anywhere, and applying transformation rules is just too tricky. The best
way to map host code to guest code in light of coding style differences
etc is using comments. You did it in case of the type values, it
applies equally here.
--
MST
Re: [PATCH] qemu-common.h: Update copyright string to include 2020
> On Mar 14, 2020, at 5:33 PM, Philippe Mathieu-Daudé wrote:
>
> Extend the copyright range to include the current year.
>
> Reported-by: John Arbuckle
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/qemu-common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 082da59e85..d0142f29ac 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -13,7 +13,7 @@
> #define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
>
> /* Copyright string for -version arguments, About dialogs, etc */
> -#define QEMU_COPYRIGHT "Copyright (c) 2003-2019 " \
> +#define QEMU_COPYRIGHT "Copyright (c) 2003-2020 " \
> "Fabrice Bellard and the QEMU Project developers"
>
> /* Bug reporting information for --help arguments, About dialogs, etc */
> --
> 2.21.1
>
Thank you for making this patch. I tried my best to apply it but I kept seeing
'malformed patch at line' errors. It was probably an issue with my email client.
Re: [PATCH 4/8] hw/arm/fsl-imx31: Wire up watchdog
On 3/14/20 6:27 PM, Guenter Roeck wrote:
With this patch, the watchdog on i.MX31 emulations is fully operational.
Signed-off-by: Guenter Roeck
---
hw/arm/fsl-imx31.c | 6 ++
include/hw/arm/fsl-imx31.h | 4
2 files changed, 10 insertions(+)
diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
index 55e90d104b..cec7d0dd1b 100644
--- a/hw/arm/fsl-imx31.c
+++ b/hw/arm/fsl-imx31.c
@@ -63,6 +63,8 @@ static void fsl_imx31_init(Object *obj)
sysbus_init_child_obj(obj, "gpio[*]", >gpio[i], sizeof(s->gpio[i]),
TYPE_IMX_GPIO);
}
+
+sysbus_init_child_obj(obj, "wdt", >wdt, sizeof(s->wdt), TYPE_IMX2_WDT);
}
static void fsl_imx31_realize(DeviceState *dev, Error **errp)
@@ -205,6 +207,10 @@ static void fsl_imx31_realize(DeviceState *dev, Error
**errp)
gpio_table[i].irq));
}
+/* Watchdog */
+object_property_set_bool(OBJECT(>wdt), true, "realized", _abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(>wdt), 0, FSL_IMX31_WDT_ADDR);
+
/* On a real system, the first 16k is a `secure boot rom' */
memory_region_init_rom(>secure_rom, NULL, "imx31.secure_rom",
FSL_IMX31_SECURE_ROM_SIZE, );
diff --git a/include/hw/arm/fsl-imx31.h b/include/hw/arm/fsl-imx31.h
index ac5ca9826a..dd8561b309 100644
--- a/include/hw/arm/fsl-imx31.h
+++ b/include/hw/arm/fsl-imx31.h
@@ -25,6 +25,7 @@
#include "hw/timer/imx_epit.h"
#include "hw/i2c/imx_i2c.h"
#include "hw/gpio/imx_gpio.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "exec/memory.h"
#include "target/arm/cpu.h"
@@ -49,6 +50,7 @@ typedef struct FslIMX31State {
IMXEPITState epit[FSL_IMX31_NUM_EPITS];
IMXI2CStatei2c[FSL_IMX31_NUM_I2CS];
IMXGPIOState gpio[FSL_IMX31_NUM_GPIOS];
+IMX2WdtState wdt;
MemoryRegion secure_rom;
MemoryRegion rom;
MemoryRegion iram;
@@ -87,6 +89,8 @@ typedef struct FslIMX31State {
#define FSL_IMX31_GPIO1_SIZE0x4000
#define FSL_IMX31_GPIO2_ADDR0x53FD
#define FSL_IMX31_GPIO2_SIZE0x4000
+#define FSL_IMX31_WDT_ADDR 0x53FDC000
+#define FSL_IMX31_WDT_SIZE 0x4000
#define FSL_IMX31_AVIC_ADDR 0x6800
#define FSL_IMX31_AVIC_SIZE 0x100
#define FSL_IMX31_SDRAM0_ADDR 0x8000
Missing Kconfig hunk:
-- >8 --
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 4cf8fa4967..8af023abde 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -366,6 +366,7 @@ config FSL_IMX31
select SERIAL
select IMX
select IMX_I2C
+select WDT_IMX2
select LAN9118
config FSL_IMX6
---
With it:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 3/8] hw/arm/fsl-imx25: Wire up watchdog
On 3/14/20 6:27 PM, Guenter Roeck wrote:
With this commit, the watchdog on imx25-pdk is fully operational,
including pretimeout support.
Signed-off-by: Guenter Roeck
---
hw/arm/fsl-imx25.c | 10 ++
include/hw/arm/fsl-imx25.h | 5 +
2 files changed, 15 insertions(+)
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index a3f829f436..7d5aab562d 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -87,6 +87,7 @@ static void fsl_imx25_init(Object *obj)
TYPE_CHIPIDEA);
}
+sysbus_init_child_obj(obj, "wdt", >wdt, sizeof(s->wdt), TYPE_IMX2_WDT);
}
static void fsl_imx25_realize(DeviceState *dev, Error **errp)
@@ -302,6 +303,15 @@ static void fsl_imx25_realize(DeviceState *dev, Error
**errp)
usb_table[i].irq));
}
+/* Watchdog */
+object_property_set_bool(OBJECT(>wdt), true, "pretimeout-support",
+ _abort);
+object_property_set_bool(OBJECT(>wdt), true, "realized", _abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(>wdt), 0, FSL_IMX25_WDT_ADDR);
+sysbus_connect_irq(SYS_BUS_DEVICE(>wdt), 0,
+ qdev_get_gpio_in(DEVICE(>avic),
+ FSL_IMX25_WDT_IRQ));
+
/* initialize 2 x 16 KB ROM */
memory_region_init_rom(>rom[0], NULL,
"imx25.rom0", FSL_IMX25_ROM0_SIZE, );
diff --git a/include/hw/arm/fsl-imx25.h b/include/hw/arm/fsl-imx25.h
index 5e196bbf05..9e228dacea 100644
--- a/include/hw/arm/fsl-imx25.h
+++ b/include/hw/arm/fsl-imx25.h
@@ -29,6 +29,7 @@
#include "hw/gpio/imx_gpio.h"
#include "hw/sd/sdhci.h"
#include "hw/usb/chipidea.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "exec/memory.h"
#include "target/arm/cpu.h"
@@ -60,6 +61,7 @@ typedef struct FslIMX25State {
IMXGPIOState gpio[FSL_IMX25_NUM_GPIOS];
SDHCIState esdhc[FSL_IMX25_NUM_ESDHCS];
ChipideaState usb[FSL_IMX25_NUM_USBS];
+IMX2WdtState wdt;
MemoryRegion rom[2];
MemoryRegion iram;
MemoryRegion iram_alias;
@@ -229,6 +231,8 @@ typedef struct FslIMX25State {
#define FSL_IMX25_GPIO1_SIZE0x4000
#define FSL_IMX25_GPIO2_ADDR0x53FD
#define FSL_IMX25_GPIO2_SIZE0x4000
+#define FSL_IMX25_WDT_ADDR 0x53FDC000
+#define FSL_IMX25_WDT_SIZE 0x4000
#define FSL_IMX25_USB1_ADDR 0x53FF4000
#define FSL_IMX25_USB1_SIZE 0x0200
#define FSL_IMX25_USB2_ADDR 0x53FF4400
@@ -268,5 +272,6 @@ typedef struct FslIMX25State {
#define FSL_IMX25_ESDHC2_IRQ8
#define FSL_IMX25_USB1_IRQ 37
#define FSL_IMX25_USB2_IRQ 35
+#define FSL_IMX25_WDT_IRQ 55
#endif /* FSL_IMX25_H */
Here you also need:
-- >8 --
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index c662d5f1e0..4cf8fa4967 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -358,6 +358,7 @@ config FSL_IMX25
select IMX
select IMX_FEC
select IMX_I2C
+select WDT_IMX2
select DS1338
config FSL_IMX31
---
With it:
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH 1/8] hw: Move i.MX watchdog driver to hw/watchdog
On 3/14/20 10:43 PM, Philippe Mathieu-Daudé wrote:
Hi Guenter,
On 3/14/20 6:27 PM, Guenter Roeck wrote:
In preparation for a full implementation, move i.MX watchdog driver
from hw/misc to hw/watchdog. While at it, add the watchdog files
to MAINTAINERS.
Signed-off-by: Guenter Roeck
---
MAINTAINERS | 2 ++
hw/misc/Makefile.objs | 1 -
hw/watchdog/Kconfig | 5 +
hw/watchdog/Makefile.objs | 1 +
hw/{misc/imx2_wdt.c => watchdog/wdt_imx2.c} | 2 +-
include/hw/arm/fsl-imx6.h | 2 +-
include/hw/arm/fsl-imx6ul.h | 2 +-
include/hw/arm/fsl-imx7.h | 2 +-
include/hw/{misc/imx2_wdt.h => watchdog/wdt_imx2.h} | 0
9 files changed, 12 insertions(+), 5 deletions(-)
rename hw/{misc/imx2_wdt.c => watchdog/wdt_imx2.c} (98%)
rename include/hw/{misc/imx2_wdt.h => watchdog/wdt_imx2.h} (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 32867bc636..d1197014e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -603,8 +603,10 @@ S: Odd Fixes
F: hw/arm/fsl-imx25.c
F: hw/arm/imx25_pdk.c
F: hw/misc/imx25_ccm.c
+F: hw/watchdog/wdt_imx2.c
F: include/hw/arm/fsl-imx25.h
F: include/hw/misc/imx25_ccm.h
+F: include/hw/watchdog/wdt_imx2.h
i.MX31 (kzm)
M: Peter Chubb
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 68aae2eabb..b25181b711 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -44,7 +44,6 @@ common-obj-$(CONFIG_IMX) += imx6_ccm.o
common-obj-$(CONFIG_IMX) += imx6ul_ccm.o
obj-$(CONFIG_IMX) += imx6_src.o
common-obj-$(CONFIG_IMX) += imx7_ccm.o
-common-obj-$(CONFIG_IMX) += imx2_wdt.o
common-obj-$(CONFIG_IMX) += imx7_snvs.o
common-obj-$(CONFIG_IMX) += imx7_gpr.o
common-obj-$(CONFIG_IMX) += imx_rngc.o
diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
index 2118d897c9..2c225b4b17 100644
--- a/hw/watchdog/Kconfig
+++ b/hw/watchdog/Kconfig
@@ -14,3 +14,8 @@ config WDT_IB700
config WDT_DIAG288
bool
+
+config WDT_IMX2
+ bool
+ default y
+ depends on IMX
- it does not depend of IMX (you could use it in another SoC)
- since it is not user-creatable, it is pointless as a default device.
Instead, select it in each iMX SoC:
-- >8 --
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index e5a876c8d1..cdb01c4e03 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -358,6 +358,7 @@ config FSL_IMX25
select IMX
select IMX_FEC
select IMX_I2C
+ select WDT_IMX2
select DS1338
config FSL_IMX31
@@ -365,6 +366,7 @@ config FSL_IMX31
select SERIAL
select IMX
select IMX_I2C
+ select WDT_IMX2
select LAN9118
config FSL_IMX6
@@ -373,6 +375,7 @@ config FSL_IMX6
select IMX
select IMX_FEC
select IMX_I2C
+ select WDT_IMX2
select SDHCI
---
Corrected hunk:
-- >8 --
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index e5a876c8d1..c662d5f1e0 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -373,6 +373,7 @@ config FSL_IMX6
select IMX
select IMX_FEC
select IMX_I2C
+select WDT_IMX2
select SDHCI
config ASPEED_SOC
@@ -410,6 +411,7 @@ config FSL_IMX7
select IMX
select IMX_FEC
select IMX_I2C
+select WDT_IMX2
select PCI_EXPRESS_DESIGNWARE
select SDHCI
select UNIMP
@@ -423,6 +425,7 @@ config FSL_IMX6UL
select IMX
select IMX_FEC
select IMX_I2C
+select WDT_IMX2
select SDHCI
select UNIMP
---
With this hunk amended (and removing "default y" and "depends on IMX"):
Reviewed-by: Philippe Mathieu-Daudé
diff --git a/hw/watchdog/Makefile.objs b/hw/watchdog/Makefile.objs
index 3f536d1cad..dd9b37f642 100644
--- a/hw/watchdog/Makefile.objs
+++ b/hw/watchdog/Makefile.objs
@@ -4,3 +4,4 @@ common-obj-$(CONFIG_WDT_IB6300ESB) += wdt_i6300esb.o
common-obj-$(CONFIG_WDT_IB700) += wdt_ib700.o
common-obj-$(CONFIG_WDT_DIAG288) += wdt_diag288.o
common-obj-$(CONFIG_ASPEED_SOC) += wdt_aspeed.o
+common-obj-$(CONFIG_WDT_IMX) += wdt_imx2.o
diff --git a/hw/misc/imx2_wdt.c b/hw/watchdog/wdt_imx2.c
similarity index 98%
rename from hw/misc/imx2_wdt.c
rename to hw/watchdog/wdt_imx2.c
index 2aedfe803a..ad1ef02e9e 100644
--- a/hw/misc/imx2_wdt.c
+++ b/hw/watchdog/wdt_imx2.c
@@ -14,7 +14,7 @@
#include "qemu/module.h"
#include "sysemu/watchdog.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#define IMX2_WDT_WCR_WDA BIT(5) /* -> External Reset WDOG_B */
#define IMX2_WDT_WCR_SRS BIT(4) /* -> Software Reset Signal */
diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index 60eadccb42..5b02dc1f4b 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -21,7 +21,7 @@
#include "hw/cpu/a9mpcore.h"
#include "hw/misc/imx6_ccm.h"
#include "hw/misc/imx6_src.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#include
Re: [PATCH 1/8] hw: Move i.MX watchdog driver to hw/watchdog
Hi Guenter,
On 3/14/20 6:27 PM, Guenter Roeck wrote:
In preparation for a full implementation, move i.MX watchdog driver
from hw/misc to hw/watchdog. While at it, add the watchdog files
to MAINTAINERS.
Signed-off-by: Guenter Roeck
---
MAINTAINERS | 2 ++
hw/misc/Makefile.objs | 1 -
hw/watchdog/Kconfig | 5 +
hw/watchdog/Makefile.objs | 1 +
hw/{misc/imx2_wdt.c => watchdog/wdt_imx2.c} | 2 +-
include/hw/arm/fsl-imx6.h | 2 +-
include/hw/arm/fsl-imx6ul.h | 2 +-
include/hw/arm/fsl-imx7.h | 2 +-
include/hw/{misc/imx2_wdt.h => watchdog/wdt_imx2.h} | 0
9 files changed, 12 insertions(+), 5 deletions(-)
rename hw/{misc/imx2_wdt.c => watchdog/wdt_imx2.c} (98%)
rename include/hw/{misc/imx2_wdt.h => watchdog/wdt_imx2.h} (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 32867bc636..d1197014e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -603,8 +603,10 @@ S: Odd Fixes
F: hw/arm/fsl-imx25.c
F: hw/arm/imx25_pdk.c
F: hw/misc/imx25_ccm.c
+F: hw/watchdog/wdt_imx2.c
F: include/hw/arm/fsl-imx25.h
F: include/hw/misc/imx25_ccm.h
+F: include/hw/watchdog/wdt_imx2.h
i.MX31 (kzm)
M: Peter Chubb
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 68aae2eabb..b25181b711 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -44,7 +44,6 @@ common-obj-$(CONFIG_IMX) += imx6_ccm.o
common-obj-$(CONFIG_IMX) += imx6ul_ccm.o
obj-$(CONFIG_IMX) += imx6_src.o
common-obj-$(CONFIG_IMX) += imx7_ccm.o
-common-obj-$(CONFIG_IMX) += imx2_wdt.o
common-obj-$(CONFIG_IMX) += imx7_snvs.o
common-obj-$(CONFIG_IMX) += imx7_gpr.o
common-obj-$(CONFIG_IMX) += imx_rngc.o
diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
index 2118d897c9..2c225b4b17 100644
--- a/hw/watchdog/Kconfig
+++ b/hw/watchdog/Kconfig
@@ -14,3 +14,8 @@ config WDT_IB700
config WDT_DIAG288
bool
+
+config WDT_IMX2
+bool
+default y
+depends on IMX
- it does not depend of IMX (you could use it in another SoC)
- since it is not user-creatable, it is pointless as a default device.
Instead, select it in each iMX SoC:
-- >8 --
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index e5a876c8d1..cdb01c4e03 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -358,6 +358,7 @@ config FSL_IMX25
select IMX
select IMX_FEC
select IMX_I2C
+select WDT_IMX2
select DS1338
config FSL_IMX31
@@ -365,6 +366,7 @@ config FSL_IMX31
select SERIAL
select IMX
select IMX_I2C
+select WDT_IMX2
select LAN9118
config FSL_IMX6
@@ -373,6 +375,7 @@ config FSL_IMX6
select IMX
select IMX_FEC
select IMX_I2C
+select WDT_IMX2
select SDHCI
---
With this hunk amended (and removing "default y" and "depends on IMX"):
Reviewed-by: Philippe Mathieu-Daudé
diff --git a/hw/watchdog/Makefile.objs b/hw/watchdog/Makefile.objs
index 3f536d1cad..dd9b37f642 100644
--- a/hw/watchdog/Makefile.objs
+++ b/hw/watchdog/Makefile.objs
@@ -4,3 +4,4 @@ common-obj-$(CONFIG_WDT_IB6300ESB) += wdt_i6300esb.o
common-obj-$(CONFIG_WDT_IB700) += wdt_ib700.o
common-obj-$(CONFIG_WDT_DIAG288) += wdt_diag288.o
common-obj-$(CONFIG_ASPEED_SOC) += wdt_aspeed.o
+common-obj-$(CONFIG_WDT_IMX) += wdt_imx2.o
diff --git a/hw/misc/imx2_wdt.c b/hw/watchdog/wdt_imx2.c
similarity index 98%
rename from hw/misc/imx2_wdt.c
rename to hw/watchdog/wdt_imx2.c
index 2aedfe803a..ad1ef02e9e 100644
--- a/hw/misc/imx2_wdt.c
+++ b/hw/watchdog/wdt_imx2.c
@@ -14,7 +14,7 @@
#include "qemu/module.h"
#include "sysemu/watchdog.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#define IMX2_WDT_WCR_WDABIT(5) /* -> External Reset WDOG_B */
#define IMX2_WDT_WCR_SRSBIT(4) /* -> Software Reset Signal */
diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index 60eadccb42..5b02dc1f4b 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -21,7 +21,7 @@
#include "hw/cpu/a9mpcore.h"
#include "hw/misc/imx6_ccm.h"
#include "hw/misc/imx6_src.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "hw/char/imx_serial.h"
#include "hw/timer/imx_gpt.h"
#include "hw/timer/imx_epit.h"
diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index eda389aec7..91c746918a 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -24,7 +24,7 @@
#include "hw/misc/imx7_snvs.h"
#include "hw/misc/imx7_gpr.h"
#include "hw/intc/imx_gpcv2.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "hw/gpio/imx_gpio.h"
#include "hw/char/imx_serial.h"
#include "hw/timer/imx_gpt.h"
diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 706aef2e7e..3a0041c4c2 100644
--- a/include/hw/arm/fsl-imx7.h
+++ b/include/hw/arm/fsl-imx7.h
Re: [PATCH 4/4] usb-serial: Fix timeout closing the device
Jason Andryuk, le jeu. 12 mars 2020 08:55:23 -0400, a ecrit: > Linux guests wait ~30 seconds when closing the emulated /dev/ttyUSB0. > During that time, the kernel driver is sending many control URBs > requesting GetModemStat (5). Real hardware returns a status with > FTDI_THRE (Transmitter Holding Register) and FTDI_TEMT (Transmitter > Empty) set. QEMU leaves them clear, and it seems Linux is waiting for > FTDI_TEMT to be set to indicate the tx queue is empty before closing. > > Set the bits when responding to a GetModemStat query and avoid the > shutdown delay. > > Signed-off-by: Jason Andryuk Reviewed-by: Samuel Thibault > --- > Looking at a USB dump for a real FTDI USB adapter, I see these bits set > in all the bulk URBs where QEMU currently has them clear. > --- > hw/usb/dev-serial.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c > index ef33bcd127..5389235f17 100644 > --- a/hw/usb/dev-serial.c > +++ b/hw/usb/dev-serial.c > @@ -332,7 +332,7 @@ static void usb_serial_handle_control(USBDevice *dev, > USBPacket *p, > break; > case DeviceInVendor | FTDI_GET_MDM_ST: > data[0] = usb_get_modem_lines(s) | 1; > -data[1] = 0; > +data[1] = FTDI_THRE | FTDI_TEMT; > p->actual_length = 2; > break; > case DeviceOutVendor | FTDI_SET_EVENT_CHR: > -- > 2.24.1 > -- Samuel c> ah (on trouve fluide glacial sur le net, ou il faut aller dans le monde reel ?) s> dans le monde reel c> zut
Re: [PATCH 3/4] usb-serial: Increase receive buffer to 496
Jason Andryuk, le jeu. 12 mars 2020 08:55:22 -0400, a ecrit:
> A FTDI USB adapter on an xHCI controller can send 512 byte USB packets.
> These are 8 * ( 2 bytes header + 62 bytes data). A 384 byte receive
> buffer is insufficient to fill a 512 byte packet, so bump the receive
> size to 496 ( 512 - 2 * 8 ).
>
> Signed-off-by: Jason Andryuk
Reviewed-by: Samuel Thibault
> ---
> hw/usb/dev-serial.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
> index 96b6c34202..ef33bcd127 100644
> --- a/hw/usb/dev-serial.c
> +++ b/hw/usb/dev-serial.c
> @@ -29,7 +29,7 @@ do { printf("usb-serial: " fmt , ## __VA_ARGS__); } while
> (0)
> #define DPRINTF(fmt, ...) do {} while(0)
> #endif
>
> -#define RECV_BUF 384
> +#define RECV_BUF (512 - (2 * 8))
>
> /* Commands */
> #define FTDI_RESET 0
> --
> 2.24.1
>
--
Samuel
After watching my newly-retired dad spend two weeks learning how to make a new
folder, it became obvious that "intuitive" mostly means "what the writer or
speaker of intuitive likes".
(Bruce Ediger, bedi...@teal.csn.org, in comp.os.linux.misc, on X the
intuitiveness of a Mac interface.)
[PATCH] qemu-common.h: Update copyright string to include 2020
Extend the copyright range to include the current year.
Reported-by: John Arbuckle
Signed-off-by: Philippe Mathieu-Daudé
---
include/qemu-common.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/qemu-common.h b/include/qemu-common.h
index 082da59e85..d0142f29ac 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -13,7 +13,7 @@
#define TFR(expr) do { if ((expr) != -1) break; } while (errno == EINTR)
/* Copyright string for -version arguments, About dialogs, etc */
-#define QEMU_COPYRIGHT "Copyright (c) 2003-2019 " \
+#define QEMU_COPYRIGHT "Copyright (c) 2003-2020 " \
"Fabrice Bellard and the QEMU Project developers"
/* Bug reporting information for --help arguments, About dialogs, etc */
--
2.21.1
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On Sat, Mar 14, 2020 at 10:05:20PM +0200, Liran Alon wrote: > Michael, you can also refer to this VMware time-keeping whitepaper: > https://www.vmware.com/pdf/vmware_timekeeping.pdf. > According to section "Initializing and Correcting Wall-Clock Time": > """ > VMware Tools can also optionally be used to correct long‐term drift and > errors by periodically > resynchronizing the virtual machine’s clock to the host’s clock, but the > current version at this writing is limited. > In particular, in guest operating systems other than NetWare, it does not > correct errors in which the guest clock > is ahead of real time, only those in which the guest clock is behind. > > """ This talks about guest time. What this does not mention is whether hosts need to employ any mechanisms to synchronise wall clock between hosts. > If I understand correctly, this seems to validate my assumption that current > implementation for CMD_GETTIME is sufficient. So I am concerned this does not interact well with other time sources in QEMU. For example, it's very useful to set guest time with -rtc base flag. Can you use qemu_get_timedate? -- MST
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On Sat, Mar 14, 2020 at 09:58:23PM +0200, Nikita Leshenko wrote: > I think that the reason that open-vm-tools doesn't move time backwards is to > help applications that treat wallclock time as if it's monotonic time and > break > if the date is moved backwards (which may happen more frequently in virtual > environment so it's handled specifically). But this doesn't change the fact > that > this PV interface is for reporting wall time. > So I couldn't understand what > source other than gettimeofday() you were suggesting for Liran to use to > report > wallclock time. Some kind of offset to wallclock time I'm guessing. For example, we could save wall clock on vm save, and if it goes backwards on vm load, add an offset. -- MST
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
> On 14 Mar 2020, at 21:26, Michael S. Tsirkin wrote:
>
> On Sat, Mar 14, 2020 at 09:17:30PM +0200, Liran Alon wrote:
>>
>> On 14/03/2020 21:14, Michael S. Tsirkin wrote:
>>> On Sat, Mar 14, 2020 at 09:04:30PM +0200, Liran Alon wrote:
On 14/03/2020 20:18, Michael S. Tsirkin wrote:
> On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
>> On 13/03/2020 17:47, Michael S. Tsirkin wrote:
>>> On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
>> @@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void
>> *opaque, uint32_t addr)
>> return ram_size;
>> }
>> +static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
>> +{
>> +X86CPU *cpu = X86_CPU(current_cpu);
>> +qemu_timeval tv;
>> +
>> +if (qemu_gettimeofday() < 0) {
>> +return UINT32_MAX;
>> +}
>> +
>> +cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
>> +cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
>> +return (uint32_t)tv.tv_sec;
>> +}
>> +
>> /* vmmouse helpers */
>> void vmmouse_get_data(uint32_t *data)
>> {
> That's a very weird thing to return to the guest.
> For example it's not monotonic across migrations.
That's the VMware PV interface... I didn't design it. :P
Regarding how it handles the fact time is not monotonic across
migrations,
see big comment at the start of services/plugins/timeSync/timeSync.c in
open-vm-tools regarding the time-sync algorithm used by VMware Tools.
Specifically:
"""
During normal operation this plugin only steps the time forward and
only if
the error is greater than one second.
>>> Looks like guest assumes this time only moves forward.
>>> So something needs to be done to avoid it moving
>>> backward across migrations.
>> Where do you see this assumption in guest code? I don't think this is
>> true.
>> Guest code seems to handle this by making sure to only step the time
>> forward.
> Exactly. So if host time moved backward e.g. by 100s, then for 100s
> time is not correcting. Which possibly vmware has a way to mitigate
> against e.g. by synchronising host time using their
> management app.
>
>> Read carefully services/plugins/timeSync/timeSync.c and point me to what
>> I'm
>> missing if you think otherwise (i.e. I missed something).
> I'm just going by what you write in a patch.
>
So guest doesn't assume that this time only moves forward...
Can you clarify then which change do you suggest making to this patch in
this regard? It seems correct to me.
i.e. The CMD_GETTIME implementation seems correct to me and it doesn't need
to do anything special to handle host time moving backwards.
-Liran
>>> I think something needs to be done to make sure host time as reported by
>>> this command does not move backwards significantly. Just forwarding
>>> gettimeofday won't cut it IMHO. Look at kvm clock for inspiration of
>>> things to do.
>>>
>> It isn't required by the command interface definition.
>> It's explicitly specified in timeSync.c that guest code handles the case
>> host time goes backwards.
>
> According to what you wrote it does not crash but also does not do
> updates. That will work well only if the time difference isn't large.
> Then with time as guest gets interrupted by host, the time difference
> shrinks and eventually you are resyncing again. And I'm guessing there
> are hypervisor management interfaces in place to make sure that is so.
> Linux is not guaranteed to have such interfaces so you have to come
> up with QEMU specific ones.
>
>
>> We are not inventing the interface, we just implement it according to it's
>> definition.
>>
>> -Liran
>
> Host time is a vague enough terminology. I suspect this implementation
> is too simplistic to cover all cases.
>
> --
> MST
I saw this discussion from the side and I just wanted to point out that as far
as I understand this interface is needed to sync *wallclock time* between the
guest and the host, mostly for user experience. There is no guarantee that it's
monotonic. Unlike TSC, we don't need to take special care with regard to live
migration or drift.
Just as NTP may report inconsistent time, it's OK if this interface is somewhat
inconsistent.
I think that the reason that open-vm-tools doesn't move time backwards is to
help applications that treat wallclock time as if it's monotonic time and break
if the date is moved backwards (which may happen more frequently in virtual
environment so it's handled specifically). But this doesn't change the fact that
this PV interface is for reporting wall time. So I couldn't understand what
source other than gettimeofday() you
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On 14/03/2020 21:58, Nikita Leshenko wrote:
On 14 Mar 2020, at 21:26, Michael S. Tsirkin wrote:
On Sat, Mar 14, 2020 at 09:17:30PM +0200, Liran Alon wrote:
On 14/03/2020 21:14, Michael S. Tsirkin wrote:
On Sat, Mar 14, 2020 at 09:04:30PM +0200, Liran Alon wrote:
On 14/03/2020 20:18, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
On 13/03/2020 17:47, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
@@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t
addr)
return ram_size;
}
+static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
+{
+X86CPU *cpu = X86_CPU(current_cpu);
+qemu_timeval tv;
+
+if (qemu_gettimeofday() < 0) {
+return UINT32_MAX;
+}
+
+cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
+cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
+return (uint32_t)tv.tv_sec;
+}
+
/* vmmouse helpers */
void vmmouse_get_data(uint32_t *data)
{
That's a very weird thing to return to the guest.
For example it's not monotonic across migrations.
That's the VMware PV interface... I didn't design it. :P
Regarding how it handles the fact time is not monotonic across migrations,
see big comment at the start of services/plugins/timeSync/timeSync.c in
open-vm-tools regarding the time-sync algorithm used by VMware Tools.
Specifically:
"""
During normal operation this plugin only steps the time forward and only if
the error is greater than one second.
Looks like guest assumes this time only moves forward.
So something needs to be done to avoid it moving
backward across migrations.
Where do you see this assumption in guest code? I don't think this is true.
Guest code seems to handle this by making sure to only step the time
forward.
Exactly. So if host time moved backward e.g. by 100s, then for 100s
time is not correcting. Which possibly vmware has a way to mitigate
against e.g. by synchronising host time using their
management app.
Read carefully services/plugins/timeSync/timeSync.c and point me to what I'm
missing if you think otherwise (i.e. I missed something).
I'm just going by what you write in a patch.
So guest doesn't assume that this time only moves forward...
Can you clarify then which change do you suggest making to this patch in
this regard? It seems correct to me.
i.e. The CMD_GETTIME implementation seems correct to me and it doesn't need
to do anything special to handle host time moving backwards.
-Liran
I think something needs to be done to make sure host time as reported by
this command does not move backwards significantly. Just forwarding
gettimeofday won't cut it IMHO. Look at kvm clock for inspiration of
things to do.
It isn't required by the command interface definition.
It's explicitly specified in timeSync.c that guest code handles the case
host time goes backwards.
According to what you wrote it does not crash but also does not do
updates. That will work well only if the time difference isn't large.
Then with time as guest gets interrupted by host, the time difference
shrinks and eventually you are resyncing again. And I'm guessing there
are hypervisor management interfaces in place to make sure that is so.
Linux is not guaranteed to have such interfaces so you have to come
up with QEMU specific ones.
We are not inventing the interface, we just implement it according to it's
definition.
-Liran
Host time is a vague enough terminology. I suspect this implementation
is too simplistic to cover all cases.
--
MST
I saw this discussion from the side and I just wanted to point out that as far
as I understand this interface is needed to sync *wallclock time* between the
guest and the host, mostly for user experience. There is no guarantee that it's
monotonic. Unlike TSC, we don't need to take special care with regard to live
migration or drift.
Just as NTP may report inconsistent time, it's OK if this interface is somewhat
inconsistent.
I think that the reason that open-vm-tools doesn't move time backwards is to
help applications that treat wallclock time as if it's monotonic time and break
if the date is moved backwards (which may happen more frequently in virtual
environment so it's handled specifically). But this doesn't change the fact that
this PV interface is for reporting wall time. So I couldn't understand what
source other than gettimeofday() you were suggesting for Liran to use to report
wallclock time.
Nikita
Michael, you can also refer to this VMware time-keeping whitepaper:
https://www.vmware.com/pdf/vmware_timekeeping.pdf.
According to section "Initializing and Correcting Wall-Clock Time":
"""
VMware Tools can also optionally be used to correct long‐term drift and
errors by periodically
resynchronizing the virtual machine’s clock to the host’s clock, but the
current version at this writing is limited.
In particular, in guest operating systems other than NetWare, it does
not
About dialog copyright date needs updating
Hi Peter, in the About dialog for QEMU the copyright date shows as 2019. Can it be updated to 2020 please? Thank you.
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On Sat, Mar 14, 2020 at 09:17:30PM +0200, Liran Alon wrote:
>
> On 14/03/2020 21:14, Michael S. Tsirkin wrote:
> > On Sat, Mar 14, 2020 at 09:04:30PM +0200, Liran Alon wrote:
> > > On 14/03/2020 20:18, Michael S. Tsirkin wrote:
> > > > On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
> > > > > On 13/03/2020 17:47, Michael S. Tsirkin wrote:
> > > > > > On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
> > > > > > > > > @@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void
> > > > > > > > > *opaque, uint32_t addr)
> > > > > > > > > return ram_size;
> > > > > > > > > }
> > > > > > > > > +static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
> > > > > > > > > +{
> > > > > > > > > +X86CPU *cpu = X86_CPU(current_cpu);
> > > > > > > > > +qemu_timeval tv;
> > > > > > > > > +
> > > > > > > > > +if (qemu_gettimeofday() < 0) {
> > > > > > > > > +return UINT32_MAX;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
> > > > > > > > > +cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
> > > > > > > > > +return (uint32_t)tv.tv_sec;
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > /* vmmouse helpers */
> > > > > > > > > void vmmouse_get_data(uint32_t *data)
> > > > > > > > > {
> > > > > > > > That's a very weird thing to return to the guest.
> > > > > > > > For example it's not monotonic across migrations.
> > > > > > > That's the VMware PV interface... I didn't design it. :P
> > > > > > > Regarding how it handles the fact time is not monotonic across
> > > > > > > migrations,
> > > > > > > see big comment at the start of
> > > > > > > services/plugins/timeSync/timeSync.c in
> > > > > > > open-vm-tools regarding the time-sync algorithm used by VMware
> > > > > > > Tools.
> > > > > > > Specifically:
> > > > > > > """
> > > > > > > During normal operation this plugin only steps the time forward
> > > > > > > and only if
> > > > > > > the error is greater than one second.
> > > > > > Looks like guest assumes this time only moves forward.
> > > > > > So something needs to be done to avoid it moving
> > > > > > backward across migrations.
> > > > > Where do you see this assumption in guest code? I don't think this is
> > > > > true.
> > > > > Guest code seems to handle this by making sure to only step the time
> > > > > forward.
> > > > Exactly. So if host time moved backward e.g. by 100s, then for 100s
> > > > time is not correcting. Which possibly vmware has a way to mitigate
> > > > against e.g. by synchronising host time using their
> > > > management app.
> > > >
> > > > > Read carefully services/plugins/timeSync/timeSync.c and point me to
> > > > > what I'm
> > > > > missing if you think otherwise (i.e. I missed something).
> > > > I'm just going by what you write in a patch.
> > > >
> > > So guest doesn't assume that this time only moves forward...
> > >
> > > Can you clarify then which change do you suggest making to this patch in
> > > this regard? It seems correct to me.
> > > i.e. The CMD_GETTIME implementation seems correct to me and it doesn't
> > > need
> > > to do anything special to handle host time moving backwards.
> > >
> > > -Liran
> > >
> > I think something needs to be done to make sure host time as reported by
> > this command does not move backwards significantly. Just forwarding
> > gettimeofday won't cut it IMHO. Look at kvm clock for inspiration of
> > things to do.
> >
> It isn't required by the command interface definition.
> It's explicitly specified in timeSync.c that guest code handles the case
> host time goes backwards.
According to what you wrote it does not crash but also does not do
updates. That will work well only if the time difference isn't large.
Then with time as guest gets interrupted by host, the time difference
shrinks and eventually you are resyncing again. And I'm guessing there
are hypervisor management interfaces in place to make sure that is so.
Linux is not guaranteed to have such interfaces so you have to come
up with QEMU specific ones.
> We are not inventing the interface, we just implement it according to it's
> definition.
>
> -Liran
Host time is a vague enough terminology. I suspect this implementation
is too simplistic to cover all cases.
--
MST
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On 14/03/2020 21:14, Michael S. Tsirkin wrote:
On Sat, Mar 14, 2020 at 09:04:30PM +0200, Liran Alon wrote:
On 14/03/2020 20:18, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
On 13/03/2020 17:47, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
@@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t
addr)
return ram_size;
}
+static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
+{
+X86CPU *cpu = X86_CPU(current_cpu);
+qemu_timeval tv;
+
+if (qemu_gettimeofday() < 0) {
+return UINT32_MAX;
+}
+
+cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
+cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
+return (uint32_t)tv.tv_sec;
+}
+
/* vmmouse helpers */
void vmmouse_get_data(uint32_t *data)
{
That's a very weird thing to return to the guest.
For example it's not monotonic across migrations.
That's the VMware PV interface... I didn't design it. :P
Regarding how it handles the fact time is not monotonic across migrations,
see big comment at the start of services/plugins/timeSync/timeSync.c in
open-vm-tools regarding the time-sync algorithm used by VMware Tools.
Specifically:
"""
During normal operation this plugin only steps the time forward and only if
the error is greater than one second.
Looks like guest assumes this time only moves forward.
So something needs to be done to avoid it moving
backward across migrations.
Where do you see this assumption in guest code? I don't think this is true.
Guest code seems to handle this by making sure to only step the time
forward.
Exactly. So if host time moved backward e.g. by 100s, then for 100s
time is not correcting. Which possibly vmware has a way to mitigate
against e.g. by synchronising host time using their
management app.
Read carefully services/plugins/timeSync/timeSync.c and point me to what I'm
missing if you think otherwise (i.e. I missed something).
I'm just going by what you write in a patch.
So guest doesn't assume that this time only moves forward...
Can you clarify then which change do you suggest making to this patch in
this regard? It seems correct to me.
i.e. The CMD_GETTIME implementation seems correct to me and it doesn't need
to do anything special to handle host time moving backwards.
-Liran
I think something needs to be done to make sure host time as reported by
this command does not move backwards significantly. Just forwarding
gettimeofday won't cut it IMHO. Look at kvm clock for inspiration of
things to do.
It isn't required by the command interface definition.
It's explicitly specified in timeSync.c that guest code handles the case
host time goes backwards.
We are not inventing the interface, we just implement it according to
it's definition.
-Liran
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On Sat, Mar 14, 2020 at 09:04:30PM +0200, Liran Alon wrote:
>
> On 14/03/2020 20:18, Michael S. Tsirkin wrote:
> > On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
> > > On 13/03/2020 17:47, Michael S. Tsirkin wrote:
> > > > On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
> > > > > > > @@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void
> > > > > > > *opaque, uint32_t addr)
> > > > > > > return ram_size;
> > > > > > > }
> > > > > > > +static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
> > > > > > > +{
> > > > > > > +X86CPU *cpu = X86_CPU(current_cpu);
> > > > > > > +qemu_timeval tv;
> > > > > > > +
> > > > > > > +if (qemu_gettimeofday() < 0) {
> > > > > > > +return UINT32_MAX;
> > > > > > > +}
> > > > > > > +
> > > > > > > +cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
> > > > > > > +cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
> > > > > > > +return (uint32_t)tv.tv_sec;
> > > > > > > +}
> > > > > > > +
> > > > > > > /* vmmouse helpers */
> > > > > > > void vmmouse_get_data(uint32_t *data)
> > > > > > > {
> > > > > > That's a very weird thing to return to the guest.
> > > > > > For example it's not monotonic across migrations.
> > > > > That's the VMware PV interface... I didn't design it. :P
> > > > > Regarding how it handles the fact time is not monotonic across
> > > > > migrations,
> > > > > see big comment at the start of services/plugins/timeSync/timeSync.c
> > > > > in
> > > > > open-vm-tools regarding the time-sync algorithm used by VMware Tools.
> > > > > Specifically:
> > > > > """
> > > > > During normal operation this plugin only steps the time forward and
> > > > > only if
> > > > > the error is greater than one second.
> > > > Looks like guest assumes this time only moves forward.
> > > > So something needs to be done to avoid it moving
> > > > backward across migrations.
> > > Where do you see this assumption in guest code? I don't think this is
> > > true.
> > > Guest code seems to handle this by making sure to only step the time
> > > forward.
> > Exactly. So if host time moved backward e.g. by 100s, then for 100s
> > time is not correcting. Which possibly vmware has a way to mitigate
> > against e.g. by synchronising host time using their
> > management app.
> >
> > > Read carefully services/plugins/timeSync/timeSync.c and point me to what
> > > I'm
> > > missing if you think otherwise (i.e. I missed something).
> > I'm just going by what you write in a patch.
> >
> So guest doesn't assume that this time only moves forward...
>
> Can you clarify then which change do you suggest making to this patch in
> this regard? It seems correct to me.
> i.e. The CMD_GETTIME implementation seems correct to me and it doesn't need
> to do anything special to handle host time moving backwards.
>
> -Liran
>
I think something needs to be done to make sure host time as reported by
this command does not move backwards significantly. Just forwarding
gettimeofday won't cut it IMHO. Look at kvm clock for inspiration of
things to do.
--
MST
Re: [PATCH v3 07/16] hw/i386/vmport: Introduce vmport.h
On 14/03/2020 20:25, Michael S. Tsirkin wrote: On Sat, Mar 14, 2020 at 09:31:31AM +0100, Philippe Mathieu-Daudé wrote: On 3/13/20 11:38 PM, Liran Alon wrote: On 13/03/2020 21:57, Philippe Mathieu-Daudé wrote: On 3/12/20 5:54 PM, Liran Alon wrote: No functional change. This is mere refactoring. Suggested-by: Michael S. Tsirkin Signed-off-by: Liran Alon --- hw/i386/pc.c | 1 + hw/i386/vmmouse.c | 1 + hw/i386/vmport.c | 1 + include/hw/i386/pc.h | 13 - include/hw/i386/vmport.h | 16 What about moving it to hw/i386/vmport.h (no under include/)? Reviewed-by: Philippe Mathieu-Daudé Can you explain the logic that separates between hw/i386/*.h to include/hw/i386/*.h? Headers in the include/hw/ namespace can be consumed by all machine targets. If this is a target-specific device, having it local to the target (hw/i386/) protect generic code (and other targets) of using it. This helps detecting wrong dependencies between components. I think it's true. However when headers were moved to include we weren't always able to do this correctly. So some i386 specific headers are under include. OK. So if I understand correctly, you also support moving this header to hw/i386/ instead of include/hw/i386/. So I will do so in v4. Thanks, -Liran
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On 14/03/2020 20:18, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
On 13/03/2020 17:47, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
@@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t
addr)
return ram_size;
}
+static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
+{
+X86CPU *cpu = X86_CPU(current_cpu);
+qemu_timeval tv;
+
+if (qemu_gettimeofday() < 0) {
+return UINT32_MAX;
+}
+
+cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
+cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
+return (uint32_t)tv.tv_sec;
+}
+
/* vmmouse helpers */
void vmmouse_get_data(uint32_t *data)
{
That's a very weird thing to return to the guest.
For example it's not monotonic across migrations.
That's the VMware PV interface... I didn't design it. :P
Regarding how it handles the fact time is not monotonic across migrations,
see big comment at the start of services/plugins/timeSync/timeSync.c in
open-vm-tools regarding the time-sync algorithm used by VMware Tools.
Specifically:
"""
During normal operation this plugin only steps the time forward and only if
the error is greater than one second.
Looks like guest assumes this time only moves forward.
So something needs to be done to avoid it moving
backward across migrations.
Where do you see this assumption in guest code? I don't think this is true.
Guest code seems to handle this by making sure to only step the time
forward.
Exactly. So if host time moved backward e.g. by 100s, then for 100s
time is not correcting. Which possibly vmware has a way to mitigate
against e.g. by synchronising host time using their
management app.
Read carefully services/plugins/timeSync/timeSync.c and point me to what I'm
missing if you think otherwise (i.e. I missed something).
I'm just going by what you write in a patch.
So guest doesn't assume that this time only moves forward...
Can you clarify then which change do you suggest making to this patch in
this regard? It seems correct to me.
i.e. The CMD_GETTIME implementation seems correct to me and it doesn't
need to do anything special to handle host time moving backwards.
-Liran
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On 14/03/2020 20:18, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
On 13/03/2020 17:47, Michael S. Tsirkin wrote:
On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
@@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void *opaque, uint32_t
addr)
return ram_size;
}
+static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
+{
+X86CPU *cpu = X86_CPU(current_cpu);
+qemu_timeval tv;
+
+if (qemu_gettimeofday() < 0) {
+return UINT32_MAX;
+}
+
+cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
+cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
+return (uint32_t)tv.tv_sec;
+}
+
/* vmmouse helpers */
void vmmouse_get_data(uint32_t *data)
{
That's a very weird thing to return to the guest.
For example it's not monotonic across migrations.
That's the VMware PV interface... I didn't design it. :P
Regarding how it handles the fact time is not monotonic across migrations,
see big comment at the start of services/plugins/timeSync/timeSync.c in
open-vm-tools regarding the time-sync algorithm used by VMware Tools.
Specifically:
"""
During normal operation this plugin only steps the time forward and only if
the error is greater than one second.
Looks like guest assumes this time only moves forward.
So something needs to be done to avoid it moving
backward across migrations.
Where do you see this assumption in guest code? I don't think this is true.
Guest code seems to handle this by making sure to only step the time
forward.
Exactly. So if host time moved backward e.g. by 100s, then for 100s
time is not correcting. Which possibly vmware has a way to mitigate
against e.g. by synchronising host time using their
management app.
Read carefully services/plugins/timeSync/timeSync.c and point me to what I'm
missing if you think otherwise (i.e. I missed something).
I'm just going by what you write in a patch.
So guest doesn't assume that this time only moves forward...
Can you clarify then which change do you suggest making to this patch in
this regard? It seems correct to me.
i.e. The CMD_GETTIME implementation seems correct to me and it doesn't
need to do anything special to handle host time moving backwards.
-Liran
Re: [PATCH v2 0/3]: acpi: Add Windows ACPI Emulated Device Table (WAET)
On Fri, Mar 13, 2020 at 08:58:30AM -0700, no-re...@patchew.org wrote: > Patchew URL: > https://patchew.org/QEMU/20200313145009.144820-1-liran.a...@oracle.com/ > > > > Hi, > > This series seems to have some coding style problems. See output below for > more information: > > Subject: [PATCH v2 0/3]: acpi: Add Windows ACPI Emulated Device Table (WAET) > Message-id: 20200313145009.144820-1-liran.a...@oracle.com > Type: series > > === 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 > Switched to a new branch 'test' > e9129fb acpi: unit-test: Update WAET ACPI Table expected binaries > 76eaa7a acpi: Add Windows ACPI Emulated Device Table (WAET) > 041dfae acpi: unit-test: Ignore diff in WAET ACPI table > > === OUTPUT BEGIN === > 1/3 Checking commit 041dfaefd37e (acpi: unit-test: Ignore diff in WAET ACPI > table) > WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? > #17: > new file mode 100644 > > total: 0 errors, 1 warnings, 3 lines checked > > Patch 1/3 has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > 2/3 Checking commit 76eaa7ac6ef4 (acpi: Add Windows ACPI Emulated Device > Table (WAET)) > ERROR: line over 90 characters > #43: FILE: hw/i386/acpi-build.c:2520: > + * Spec: > http://download.microsoft.com/download/7/E/7/7E7662CF-CBEA-470B-A97E-CE7CE0D98DC2/WAET.docx > > WARNING: Block comments use a leading /* on a separate line > #61: FILE: hw/i386/acpi-build.c:2538: > +build_append_int_noprefix(table_data, 1 << 1 /* ACPI PM timer good */, > 4); Looks like a false positive - this is not a block comment. What's going on? > total: 1 errors, 1 warnings, 43 lines checked > > Patch 2/3 has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > > 3/3 Checking commit e9129fbd5cf2 (acpi: unit-test: Update WAET ACPI Table > expected binaries) > === OUTPUT END === > > Test command exited with code: 1 > > > The full log is available at > http://patchew.org/logs/20200313145009.144820-1-liran.a...@oracle.com/testing.checkpatch/?type=message. > --- > Email generated automatically by Patchew [https://patchew.org/]. > Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v3 07/16] hw/i386/vmport: Introduce vmport.h
On Sat, Mar 14, 2020 at 09:31:31AM +0100, Philippe Mathieu-Daudé wrote: > On 3/13/20 11:38 PM, Liran Alon wrote: > > On 13/03/2020 21:57, Philippe Mathieu-Daudé wrote: > > > On 3/12/20 5:54 PM, Liran Alon wrote: > > > > No functional change. This is mere refactoring. > > > > > > > > Suggested-by: Michael S. Tsirkin > > > > Signed-off-by: Liran Alon > > > > --- > > > > hw/i386/pc.c | 1 + > > > > hw/i386/vmmouse.c | 1 + > > > > hw/i386/vmport.c | 1 + > > > > include/hw/i386/pc.h | 13 - > > > > include/hw/i386/vmport.h | 16 > > > > > > What about moving it to hw/i386/vmport.h (no under include/)? > > > > > > Reviewed-by: Philippe Mathieu-Daudé > > > > > > > > Can you explain the logic that separates between hw/i386/*.h to > > include/hw/i386/*.h? > > Headers in the include/hw/ namespace can be consumed by all machine targets. > If this is a target-specific device, having it local to the target > (hw/i386/) protect generic code (and other targets) of using it. This helps > detecting wrong dependencies between components. I think it's true. However when headers were moved to include we weren't always able to do this correctly. So some i386 specific headers are under include. > > If it makes sense, sure I will move it. I just don't know what is the > > convention here. > > Michael/Paolo/Eduardo what do you recommend?
Re: [PATCH V2] vhost: correctly turn on VIRTIO_F_IOMMU_PLATFORM
On Fri, Mar 13, 2020 at 03:27:59PM -0500, Brijesh Singh wrote:
>
> On 3/13/20 7:44 AM, Halil Pasic wrote:
> > [..]
> >>> CCing Tom. @Tom does vhost-vsock work for you with SEV and current qemu?
> >>>
> >>> Also, one can specify iommu_platform=on on a device that ain't a part of
> >>> a secure-capable VM, just for the fun of it. And that breaks
> >>> vhost-vsock. Or is setting iommu_platform=on only valid if
> >>> qemu-system-s390x is protected virtualization capable?
> >>>
> >>> BTW, I don't have a strong opinion on the fixes tag. We currently do not
> >>> recommend setting iommu_platform, and thus I don't think we care too
> >>> much about past qemus having problems with it.
> >>>
> >>> Regards,
> >>> Halil
> >>
> >> Let's just say if we do have a Fixes: tag we want to set it correctly to
> >> the commit that needs this fix.
> >>
> > I finally did some digging regarding the performance degradation. For
> > s390x the performance degradation on vhost-net was introduced by commit
> > 076a93d797 ("exec: simplify address_space_get_iotlb_entry"). Before
> > IOMMUTLBEntry.addr_mask used to be based on plen, which in turn was
> > calculated as the rest of the memory regions size (from address), and
> > covered most of the guest address space. That is we didn't have a whole
> > lot of IOTLB API overhead.
> >
> > With commit 076a93d797 I see IOMMUTLBEntry.addr_mask == 0xfff which comes
> > as ~TARGET_PAGE_MASK from flatview_do_translate(). To have things working
> > properly I applied 75e5b70e6, b021d1c044, and d542800d1e on the level of
> > 076a93d797 and 076a93d797~1.
> >
> > Regarding vhost-vsock. It does not work with iommu_platform=on since the
> > very beginning (i.e. 8607f5c307 ("virtio: convert to use DMA api")). Not
> > sure if that is a good or a bad thing. (If the vhost driver in the kernel
> > would actually have to do the IOTLB translation, then failing in case
> > where it does not support it seems sane. The problem is that
> > ACCESS_PLATFORM is used for more than one thing (needs translation, and
> > restricted memory access).)
> >
> > I don't think I've heard back from AMD whether vsock works with SEV or
> > not... I don't have access to HW to test it myself.
>
>
> I just tried vhost-vsock on AMD SEV machine and it does not work. I am
> using FC31 (qemu 4.1.1.1.fc31).
Neither does vhost scsi - no ACCESS_PLATFORM support. But with Jason's
patch I think both should work. Pls give it a try.
>
> > We (s390) don't require this being backported to the stable qemus,
> > because for us iommu_platform=on becomes relevant with protected
> > virtualization, and those qemu versions don't support it.
> >
> > Cheers,
> > Halil
> >
Re: [PATCH v3 10/16] hw/i386/vmport: Add support for CMD_GETTIME
On Fri, Mar 13, 2020 at 06:26:54PM +0200, Liran Alon wrote:
>
> On 13/03/2020 17:47, Michael S. Tsirkin wrote:
> > On Fri, Mar 13, 2020 at 05:25:20PM +0200, Liran Alon wrote:
> > > > > @@ -168,6 +169,20 @@ static uint32_t vmport_cmd_ram_size(void
> > > > > *opaque, uint32_t addr)
> > > > >return ram_size;
> > > > >}
> > > > > +static uint32_t vmport_cmd_time(void *opaque, uint32_t addr)
> > > > > +{
> > > > > +X86CPU *cpu = X86_CPU(current_cpu);
> > > > > +qemu_timeval tv;
> > > > > +
> > > > > +if (qemu_gettimeofday() < 0) {
> > > > > +return UINT32_MAX;
> > > > > +}
> > > > > +
> > > > > +cpu->env.regs[R_EBX] = (uint32_t)tv.tv_usec;
> > > > > +cpu->env.regs[R_ECX] = port_state->max_time_lag_us;
> > > > > +return (uint32_t)tv.tv_sec;
> > > > > +}
> > > > > +
> > > > >/* vmmouse helpers */
> > > > >void vmmouse_get_data(uint32_t *data)
> > > > >{
> > > > That's a very weird thing to return to the guest.
> > > > For example it's not monotonic across migrations.
> > > That's the VMware PV interface... I didn't design it. :P
> > > Regarding how it handles the fact time is not monotonic across migrations,
> > > see big comment at the start of services/plugins/timeSync/timeSync.c in
> > > open-vm-tools regarding the time-sync algorithm used by VMware Tools.
> > > Specifically:
> > > """
> > > During normal operation this plugin only steps the time forward and only
> > > if
> > > the error is greater than one second.
> > Looks like guest assumes this time only moves forward.
> > So something needs to be done to avoid it moving
> > backward across migrations.
> Where do you see this assumption in guest code? I don't think this is true.
> Guest code seems to handle this by making sure to only step the time
> forward.
Exactly. So if host time moved backward e.g. by 100s, then for 100s
time is not correcting. Which possibly vmware has a way to mitigate
against e.g. by synchronising host time using their
management app.
> Read carefully services/plugins/timeSync/timeSync.c and point me to what I'm
> missing if you think otherwise (i.e. I missed something).
I'm just going by what you write in a patch.
> > > """
> > > > And what does max_time_lag_us refer to, anyway?
> > > According to the comment in open-vm-tools TimeSyncReadHost():
> > > """
> > > maximum time lag allowed (config option), which is a threshold that keeps
> > > the tools from being over eager about resetting the time when it is only a
> > > little bit off.
> > > """
> > >
> > > Looking at open-vm-tools timeSync.c code, it defines the threshold of how
> > > far guest time can be from host time before deciding to do a "step
> > > correction".
> > > A "step correction" is defined as explicitly setting the time in the guest
> > > to the time in the host.
> > > >
> > > > So please add documentation about what this does.
> > > You are right. I agree.
> > > I think it would be best to just point to open-vm-tools
> > > services/plugins/timeSync/timeSync.c.
> > > Do you agree or should I copy some paragraphs from there?
> > Neither. Their documentation will be from guest point of view. Please
> > look at that code and write documentation from host point of view.
> > Your documentation for the lag parameter is I think a good
> > example of how to do it.
> Ok. Will try to phrase something for v4.
> >
> > > > If there's no document to refer to then pls write
> > > > code comments or a document under docs/ - this does not
> > > > belong in commit log.
> > > >
> > > >
> > > >
> > > > > @@ -214,6 +229,7 @@ static void vmport_realizefn(DeviceState *dev,
> > > > > Error **errp)
> > > > >vmport_register(VMPORT_CMD_GETRAMSIZE, vmport_cmd_ram_size,
> > > > > NULL);
> > > > >if (s->compat_flags & VMPORT_COMPAT_CMDS_V2) {
> > > > >vmport_register(VMPORT_CMD_GETBIOSUUID,
> > > > > vmport_cmd_get_bios_uuid, NULL);
> > > > > +vmport_register(VMPORT_CMD_GETTIME, vmport_cmd_time, NULL);
> > > > >}
> > > > >}
> > > > > @@ -249,6 +265,11 @@ static Property vmport_properties[] = {
> > > > > * 5 - ACE 1.x (Deprecated)
> > > > > */
> > > > >DEFINE_PROP_UINT8("vmware-vmx-type", VMPortState,
> > > > > vmware_vmx_type, 2),
> > > > > +/*
> > > > > + * Max amount of time lag that can go uncorrected.
> > > > What does uncorrected mean?
> > > You are right this is a bad phrasing taken from open-vm-tools.
> > > It should mean "How far we allow guest time to go from host time before
> > > guest VMware Tools will sync it to host time".
> > > How you prefer to phrase this?
> > Sounds like a good explanation. Maybe we allow -> can
> > since "we" is hypervisor and it's actually under guest control.
> Ok. Will add this to v4.
> >
> >
> > > > > + * Value taken from VMware Workstation 5.5.
> > > > How do we know this makes sense for KVM? That has significantly
> > > > different runtime characteristics.
> > > This is just a threshold as
Re: [PATCH] linux-user: Update TASK_UNMAPPED_BASE for aarch64
Le 14/03/2020 à 18:01, Aleksandar Markovic a écrit : > On Sat, Mar 14, 2020 at 11:45 AM Laurent Vivier wrote: >> >> Le 14/03/2020 à 04:06, Aleksandar Markovic a écrit : >>> On Fri, Mar 13, 2020 at 1:28 AM Lirong Yuan wrote: This change updates TASK_UNMAPPED_BASE (the base address for guest programs) for aarch64. It is needed to allow qemu to work with Thread Sanitizer (TSan), which has specific boundary definitions for memory mappings on different platforms: https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/tsan/rtl/tsan_platform.h Signed-off-by: Lirong Yuan --- linux-user/mmap.c | 4 1 file changed, 4 insertions(+) diff --git a/linux-user/mmap.c b/linux-user/mmap.c index 8685f02e7e..e378033797 100644 --- a/linux-user/mmap.c +++ b/linux-user/mmap.c @@ -184,7 +184,11 @@ static int mmap_frag(abi_ulong real_start, } #if HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64 +#ifdef TARGET_AARCH64 +# define TASK_UNMAPPED_BASE 0x55 >>> >>> Hi, Lirong, >>> >>> Can you point from which line of the file you linked to did you >>> arrive to the value 0x55? >>> >>> Second question: What about other targets? >> >> Personally, I prefer to not change the value for other targets if it is >> not required by someone that had some problems with the current value. >> >> It needs to be changed carefully and to be well tested after change. >> > > Sure, but again, from where " 0x55" comes from? The URL is in the comment, but more precisely I guess: https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/tsan/rtl/tsan_platform.h#L164 Thanks, Laurent
[PATCH 7/8] hw/arm/fsl-imx7: Instantiate various unimplemented devices
Instantiating PWM, CAN, CAAM, and OCOTP devices is necessary to avoid
crashes when booting mainline Linux.
Signed-off-by: Guenter Roeck
---
hw/arm/fsl-imx7.c | 24
include/hw/arm/fsl-imx7.h | 16
2 files changed, 40 insertions(+)
diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index 119b281a50..29382776b2 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -459,6 +459,30 @@ static void fsl_imx7_realize(DeviceState *dev, Error
**errp)
*/
create_unimplemented_device("sdma", FSL_IMX7_SDMA_ADDR,
FSL_IMX7_SDMA_SIZE);
+/*
+ * CAAM
+ */
+create_unimplemented_device("caam", FSL_IMX7_CAAM_ADDR,
FSL_IMX7_CAAM_SIZE);
+
+/*
+ * PWM
+ */
+create_unimplemented_device("pwm1", FSL_IMX7_PWM1_ADDR,
FSL_IMX7_PWMn_SIZE);
+create_unimplemented_device("pwm2", FSL_IMX7_PWM2_ADDR,
FSL_IMX7_PWMn_SIZE);
+create_unimplemented_device("pwm3", FSL_IMX7_PWM3_ADDR,
FSL_IMX7_PWMn_SIZE);
+create_unimplemented_device("pwm4", FSL_IMX7_PWM4_ADDR,
FSL_IMX7_PWMn_SIZE);
+
+/*
+ * CAN
+ */
+create_unimplemented_device("can1", FSL_IMX7_CAN1_ADDR,
FSL_IMX7_CANn_SIZE);
+create_unimplemented_device("can2", FSL_IMX7_CAN2_ADDR,
FSL_IMX7_CANn_SIZE);
+
+/*
+ * OCOTP
+ */
+create_unimplemented_device("octop", FSL_IMX7_OCOTP_ADDR,
+FSL_IMX7_OCOTP_SIZE);
object_property_set_bool(OBJECT(>gpr), true, "realized",
_abort);
diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 3a0041c4c2..47826da2b7 100644
--- a/include/hw/arm/fsl-imx7.h
+++ b/include/hw/arm/fsl-imx7.h
@@ -113,6 +113,9 @@ enum FslIMX7MemoryMap {
FSL_IMX7_IOMUXC_GPR_ADDR = 0x3034,
FSL_IMX7_IOMUXCn_SIZE = 0x1000,
+FSL_IMX7_OCOTP_ADDR = 0x3035,
+FSL_IMX7_OCOTP_SIZE = 0x1,
+
FSL_IMX7_ANALOG_ADDR = 0x3036,
FSL_IMX7_SNVS_ADDR= 0x3037,
FSL_IMX7_CCM_ADDR = 0x3038,
@@ -124,11 +127,24 @@ enum FslIMX7MemoryMap {
FSL_IMX7_ADC2_ADDR= 0x3062,
FSL_IMX7_ADCn_SIZE= 0x1000,
+FSL_IMX7_PWM1_ADDR= 0x3066,
+FSL_IMX7_PWM2_ADDR= 0x3067,
+FSL_IMX7_PWM3_ADDR= 0x3068,
+FSL_IMX7_PWM4_ADDR= 0x3069,
+FSL_IMX7_PWMn_SIZE= 0x1,
+
FSL_IMX7_PCIE_PHY_ADDR= 0x306D,
FSL_IMX7_PCIE_PHY_SIZE= 0x1,
FSL_IMX7_GPC_ADDR = 0x303A,
+FSL_IMX7_CAAM_ADDR= 0x3090,
+FSL_IMX7_CAAM_SIZE= 0x4,
+
+FSL_IMX7_CAN1_ADDR= 0x30A0,
+FSL_IMX7_CAN2_ADDR= 0x30A1,
+FSL_IMX7_CANn_SIZE= 0x1,
+
FSL_IMX7_I2C1_ADDR= 0x30A2,
FSL_IMX7_I2C2_ADDR= 0x30A3,
FSL_IMX7_I2C3_ADDR= 0x30A4,
--
2.17.1
[PATCH 6/8] hw/arm/fsl-imx6ul: Connect watchdog interrupts
With this commit, the watchdog on mcimx6ul-evk is fully operational,
including pretimeout support.
Signed-off-by: Guenter Roeck
---
hw/arm/fsl-imx6ul.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/hw/arm/fsl-imx6ul.c b/hw/arm/fsl-imx6ul.c
index c405b68d1d..8d3b91dd85 100644
--- a/hw/arm/fsl-imx6ul.c
+++ b/hw/arm/fsl-imx6ul.c
@@ -496,12 +496,22 @@ static void fsl_imx6ul_realize(DeviceState *dev, Error
**errp)
FSL_IMX6UL_WDOG2_ADDR,
FSL_IMX6UL_WDOG3_ADDR,
};
+static const int FSL_IMX6UL_WDOGn_IRQ[FSL_IMX6UL_NUM_WDTS] = {
+FSL_IMX6UL_WDOG1_IRQ,
+FSL_IMX6UL_WDOG2_IRQ,
+FSL_IMX6UL_WDOG3_IRQ,
+};
+object_property_set_bool(OBJECT(>wdt[i]), true,
"pretimeout-support",
+ _abort);
object_property_set_bool(OBJECT(>wdt[i]), true, "realized",
_abort);
sysbus_mmio_map(SYS_BUS_DEVICE(>wdt[i]), 0,
FSL_IMX6UL_WDOGn_ADDR[i]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>wdt[i]), 0,
+ qdev_get_gpio_in(DEVICE(>a7mpcore),
+FSL_IMX6UL_WDOGn_IRQ[i]));
}
/*
--
2.17.1
[PATCH 4/8] hw/arm/fsl-imx31: Wire up watchdog
With this patch, the watchdog on i.MX31 emulations is fully operational.
Signed-off-by: Guenter Roeck
---
hw/arm/fsl-imx31.c | 6 ++
include/hw/arm/fsl-imx31.h | 4
2 files changed, 10 insertions(+)
diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
index 55e90d104b..cec7d0dd1b 100644
--- a/hw/arm/fsl-imx31.c
+++ b/hw/arm/fsl-imx31.c
@@ -63,6 +63,8 @@ static void fsl_imx31_init(Object *obj)
sysbus_init_child_obj(obj, "gpio[*]", >gpio[i], sizeof(s->gpio[i]),
TYPE_IMX_GPIO);
}
+
+sysbus_init_child_obj(obj, "wdt", >wdt, sizeof(s->wdt), TYPE_IMX2_WDT);
}
static void fsl_imx31_realize(DeviceState *dev, Error **errp)
@@ -205,6 +207,10 @@ static void fsl_imx31_realize(DeviceState *dev, Error
**errp)
gpio_table[i].irq));
}
+/* Watchdog */
+object_property_set_bool(OBJECT(>wdt), true, "realized", _abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(>wdt), 0, FSL_IMX31_WDT_ADDR);
+
/* On a real system, the first 16k is a `secure boot rom' */
memory_region_init_rom(>secure_rom, NULL, "imx31.secure_rom",
FSL_IMX31_SECURE_ROM_SIZE, );
diff --git a/include/hw/arm/fsl-imx31.h b/include/hw/arm/fsl-imx31.h
index ac5ca9826a..dd8561b309 100644
--- a/include/hw/arm/fsl-imx31.h
+++ b/include/hw/arm/fsl-imx31.h
@@ -25,6 +25,7 @@
#include "hw/timer/imx_epit.h"
#include "hw/i2c/imx_i2c.h"
#include "hw/gpio/imx_gpio.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "exec/memory.h"
#include "target/arm/cpu.h"
@@ -49,6 +50,7 @@ typedef struct FslIMX31State {
IMXEPITState epit[FSL_IMX31_NUM_EPITS];
IMXI2CStatei2c[FSL_IMX31_NUM_I2CS];
IMXGPIOState gpio[FSL_IMX31_NUM_GPIOS];
+IMX2WdtState wdt;
MemoryRegion secure_rom;
MemoryRegion rom;
MemoryRegion iram;
@@ -87,6 +89,8 @@ typedef struct FslIMX31State {
#define FSL_IMX31_GPIO1_SIZE0x4000
#define FSL_IMX31_GPIO2_ADDR0x53FD
#define FSL_IMX31_GPIO2_SIZE0x4000
+#define FSL_IMX31_WDT_ADDR 0x53FDC000
+#define FSL_IMX31_WDT_SIZE 0x4000
#define FSL_IMX31_AVIC_ADDR 0x6800
#define FSL_IMX31_AVIC_SIZE 0x100
#define FSL_IMX31_SDRAM0_ADDR 0x8000
--
2.17.1
[PATCH 5/8] hw/arm/fsl-imx6: Connect watchdog interrupts
With this patch applied, the watchdog in the sabrelite emulation
is fully operational, including pretimeout support.
Signed-off-by: Guenter Roeck
---
hw/arm/fsl-imx6.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index ecc62855f2..b266d40881 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -397,11 +397,20 @@ static void fsl_imx6_realize(DeviceState *dev, Error
**errp)
FSL_IMX6_WDOG1_ADDR,
FSL_IMX6_WDOG2_ADDR,
};
+static const int FSL_IMX6_WDOGn_IRQ[FSL_IMX6_NUM_WDTS] = {
+FSL_IMX6_WDOG1_IRQ,
+FSL_IMX6_WDOG2_IRQ,
+};
+object_property_set_bool(OBJECT(>wdt[i]), true,
"pretimeout-support",
+ _abort);
object_property_set_bool(OBJECT(>wdt[i]), true, "realized",
_abort);
sysbus_mmio_map(SYS_BUS_DEVICE(>wdt[i]), 0, FSL_IMX6_WDOGn_ADDR[i]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>wdt[i]), 0,
+ qdev_get_gpio_in(DEVICE(>a9mpcore),
+FSL_IMX6_WDOGn_IRQ[i]));
}
/* ROM memory */
--
2.17.1
[PATCH 8/8] hw/arm/fsl-imx7: Connect watchdog interrupts
i.MX7 supports watchdog pretimeout interupts. With this commit,
the watchdog in mcimx7d-sabre is fully operational, including
pretimeout support.
Signed-off-by: Guenter Roeck
---
hw/arm/fsl-imx7.c | 11 +++
include/hw/arm/fsl-imx7.h | 5 +
2 files changed, 16 insertions(+)
diff --git a/hw/arm/fsl-imx7.c b/hw/arm/fsl-imx7.c
index 29382776b2..6c539c8b23 100644
--- a/hw/arm/fsl-imx7.c
+++ b/hw/arm/fsl-imx7.c
@@ -447,11 +447,22 @@ static void fsl_imx7_realize(DeviceState *dev, Error
**errp)
FSL_IMX7_WDOG3_ADDR,
FSL_IMX7_WDOG4_ADDR,
};
+static const int FSL_IMX7_WDOGn_IRQ[FSL_IMX7_NUM_WDTS] = {
+FSL_IMX7_WDOG1_IRQ,
+FSL_IMX7_WDOG2_IRQ,
+FSL_IMX7_WDOG3_IRQ,
+FSL_IMX7_WDOG4_IRQ,
+};
+object_property_set_bool(OBJECT(>wdt[i]), true,
"pretimeout-support",
+ _abort);
object_property_set_bool(OBJECT(>wdt[i]), true, "realized",
_abort);
sysbus_mmio_map(SYS_BUS_DEVICE(>wdt[i]), 0, FSL_IMX7_WDOGn_ADDR[i]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>wdt[i]), 0,
+ qdev_get_gpio_in(DEVICE(>a7mpcore),
+FSL_IMX7_WDOGn_IRQ[i]));
}
/*
diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 47826da2b7..da977f9ffb 100644
--- a/include/hw/arm/fsl-imx7.h
+++ b/include/hw/arm/fsl-imx7.h
@@ -228,6 +228,11 @@ enum FslIMX7IRQs {
FSL_IMX7_USB2_IRQ = 42,
FSL_IMX7_USB3_IRQ = 40,
+FSL_IMX7_WDOG1_IRQ= 78,
+FSL_IMX7_WDOG2_IRQ= 79,
+FSL_IMX7_WDOG3_IRQ= 10,
+FSL_IMX7_WDOG4_IRQ= 109,
+
FSL_IMX7_PCI_INTA_IRQ = 125,
FSL_IMX7_PCI_INTB_IRQ = 124,
FSL_IMX7_PCI_INTC_IRQ = 123,
--
2.17.1
[PATCH 3/8] hw/arm/fsl-imx25: Wire up watchdog
With this commit, the watchdog on imx25-pdk is fully operational,
including pretimeout support.
Signed-off-by: Guenter Roeck
---
hw/arm/fsl-imx25.c | 10 ++
include/hw/arm/fsl-imx25.h | 5 +
2 files changed, 15 insertions(+)
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index a3f829f436..7d5aab562d 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -87,6 +87,7 @@ static void fsl_imx25_init(Object *obj)
TYPE_CHIPIDEA);
}
+sysbus_init_child_obj(obj, "wdt", >wdt, sizeof(s->wdt), TYPE_IMX2_WDT);
}
static void fsl_imx25_realize(DeviceState *dev, Error **errp)
@@ -302,6 +303,15 @@ static void fsl_imx25_realize(DeviceState *dev, Error
**errp)
usb_table[i].irq));
}
+/* Watchdog */
+object_property_set_bool(OBJECT(>wdt), true, "pretimeout-support",
+ _abort);
+object_property_set_bool(OBJECT(>wdt), true, "realized", _abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(>wdt), 0, FSL_IMX25_WDT_ADDR);
+sysbus_connect_irq(SYS_BUS_DEVICE(>wdt), 0,
+ qdev_get_gpio_in(DEVICE(>avic),
+ FSL_IMX25_WDT_IRQ));
+
/* initialize 2 x 16 KB ROM */
memory_region_init_rom(>rom[0], NULL,
"imx25.rom0", FSL_IMX25_ROM0_SIZE, );
diff --git a/include/hw/arm/fsl-imx25.h b/include/hw/arm/fsl-imx25.h
index 5e196bbf05..9e228dacea 100644
--- a/include/hw/arm/fsl-imx25.h
+++ b/include/hw/arm/fsl-imx25.h
@@ -29,6 +29,7 @@
#include "hw/gpio/imx_gpio.h"
#include "hw/sd/sdhci.h"
#include "hw/usb/chipidea.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "exec/memory.h"
#include "target/arm/cpu.h"
@@ -60,6 +61,7 @@ typedef struct FslIMX25State {
IMXGPIOState gpio[FSL_IMX25_NUM_GPIOS];
SDHCIState esdhc[FSL_IMX25_NUM_ESDHCS];
ChipideaState usb[FSL_IMX25_NUM_USBS];
+IMX2WdtState wdt;
MemoryRegion rom[2];
MemoryRegion iram;
MemoryRegion iram_alias;
@@ -229,6 +231,8 @@ typedef struct FslIMX25State {
#define FSL_IMX25_GPIO1_SIZE0x4000
#define FSL_IMX25_GPIO2_ADDR0x53FD
#define FSL_IMX25_GPIO2_SIZE0x4000
+#define FSL_IMX25_WDT_ADDR 0x53FDC000
+#define FSL_IMX25_WDT_SIZE 0x4000
#define FSL_IMX25_USB1_ADDR 0x53FF4000
#define FSL_IMX25_USB1_SIZE 0x0200
#define FSL_IMX25_USB2_ADDR 0x53FF4400
@@ -268,5 +272,6 @@ typedef struct FslIMX25State {
#define FSL_IMX25_ESDHC2_IRQ8
#define FSL_IMX25_USB1_IRQ 37
#define FSL_IMX25_USB2_IRQ 35
+#define FSL_IMX25_WDT_IRQ 55
#endif /* FSL_IMX25_H */
--
2.17.1
[PATCH 0/8] hw/arm: Implement i.MX watchdog support
The current i.MX watchdog implementation only supports resets. This patch series implements the full watchdog, including optional pretimeout support. Notable changes: - The existing i.MX watchdog emulation (which only emulates syste resets) is moved from hw/misc to hw/watchdog and renamed to match the naming convention in hw/watchdog (patch 1/8). - Full watchdog support is implemented in patch 2/8. - The watchdog is wired up for i.MX25 and i.MX31 emulations (patch 3/8 and 4/8). - The watchdog interrupt (for pretimeout support) is connected for i.MX6, i.MX6UL, and i.MX7 emulations (patch 5/8, 6/8, and 8/8). - For i.MX7, various devices are wired up as unimplemented devices (patch 7/8). This was necessary to avoid crashes when booting recent Linux kernels. The code was tested with all available emulations. Guenter Roeck (8): hw: Move i.MX watchdog driver to hw/watchdog hw/watchdog: Implement full i.MX watchdog support hw/arm/fsl-imx25: Wire up watchdog hw/arm/fsl-imx31: Wire up watchdog hw/arm/fsl-imx6: Connect watchdog interrupts hw/arm/fsl-imx6ul: Connect watchdog interrupts hw/arm/fsl-imx7: Instantiate various unimplemented devices hw/arm/fsl-imx7: Connect watchdog interrupts MAINTAINERS| 2 + hw/arm/fsl-imx25.c | 10 ++ hw/arm/fsl-imx31.c | 6 + hw/arm/fsl-imx6.c | 9 ++ hw/arm/fsl-imx6ul.c| 10 ++ hw/arm/fsl-imx7.c | 35 ++ hw/misc/Makefile.objs | 1 - hw/misc/imx2_wdt.c | 90 -- hw/watchdog/Kconfig| 5 + hw/watchdog/Makefile.objs | 1 + hw/watchdog/wdt_imx2.c | 262 + include/hw/arm/fsl-imx25.h | 5 + include/hw/arm/fsl-imx31.h | 4 + include/hw/arm/fsl-imx6.h | 2 +- include/hw/arm/fsl-imx6ul.h| 2 +- include/hw/arm/fsl-imx7.h | 23 +++- include/hw/misc/imx2_wdt.h | 33 -- include/hw/watchdog/wdt_imx2.h | 78 18 files changed, 451 insertions(+), 127 deletions(-) delete mode 100644 hw/misc/imx2_wdt.c create mode 100644 hw/watchdog/wdt_imx2.c delete mode 100644 include/hw/misc/imx2_wdt.h create mode 100644 include/hw/watchdog/wdt_imx2.h
[PATCH 1/8] hw: Move i.MX watchdog driver to hw/watchdog
In preparation for a full implementation, move i.MX watchdog driver
from hw/misc to hw/watchdog. While at it, add the watchdog files
to MAINTAINERS.
Signed-off-by: Guenter Roeck
---
MAINTAINERS | 2 ++
hw/misc/Makefile.objs | 1 -
hw/watchdog/Kconfig | 5 +
hw/watchdog/Makefile.objs | 1 +
hw/{misc/imx2_wdt.c => watchdog/wdt_imx2.c} | 2 +-
include/hw/arm/fsl-imx6.h | 2 +-
include/hw/arm/fsl-imx6ul.h | 2 +-
include/hw/arm/fsl-imx7.h | 2 +-
include/hw/{misc/imx2_wdt.h => watchdog/wdt_imx2.h} | 0
9 files changed, 12 insertions(+), 5 deletions(-)
rename hw/{misc/imx2_wdt.c => watchdog/wdt_imx2.c} (98%)
rename include/hw/{misc/imx2_wdt.h => watchdog/wdt_imx2.h} (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index 32867bc636..d1197014e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -603,8 +603,10 @@ S: Odd Fixes
F: hw/arm/fsl-imx25.c
F: hw/arm/imx25_pdk.c
F: hw/misc/imx25_ccm.c
+F: hw/watchdog/wdt_imx2.c
F: include/hw/arm/fsl-imx25.h
F: include/hw/misc/imx25_ccm.h
+F: include/hw/watchdog/wdt_imx2.h
i.MX31 (kzm)
M: Peter Chubb
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 68aae2eabb..b25181b711 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -44,7 +44,6 @@ common-obj-$(CONFIG_IMX) += imx6_ccm.o
common-obj-$(CONFIG_IMX) += imx6ul_ccm.o
obj-$(CONFIG_IMX) += imx6_src.o
common-obj-$(CONFIG_IMX) += imx7_ccm.o
-common-obj-$(CONFIG_IMX) += imx2_wdt.o
common-obj-$(CONFIG_IMX) += imx7_snvs.o
common-obj-$(CONFIG_IMX) += imx7_gpr.o
common-obj-$(CONFIG_IMX) += imx_rngc.o
diff --git a/hw/watchdog/Kconfig b/hw/watchdog/Kconfig
index 2118d897c9..2c225b4b17 100644
--- a/hw/watchdog/Kconfig
+++ b/hw/watchdog/Kconfig
@@ -14,3 +14,8 @@ config WDT_IB700
config WDT_DIAG288
bool
+
+config WDT_IMX2
+bool
+default y
+depends on IMX
diff --git a/hw/watchdog/Makefile.objs b/hw/watchdog/Makefile.objs
index 3f536d1cad..dd9b37f642 100644
--- a/hw/watchdog/Makefile.objs
+++ b/hw/watchdog/Makefile.objs
@@ -4,3 +4,4 @@ common-obj-$(CONFIG_WDT_IB6300ESB) += wdt_i6300esb.o
common-obj-$(CONFIG_WDT_IB700) += wdt_ib700.o
common-obj-$(CONFIG_WDT_DIAG288) += wdt_diag288.o
common-obj-$(CONFIG_ASPEED_SOC) += wdt_aspeed.o
+common-obj-$(CONFIG_WDT_IMX) += wdt_imx2.o
diff --git a/hw/misc/imx2_wdt.c b/hw/watchdog/wdt_imx2.c
similarity index 98%
rename from hw/misc/imx2_wdt.c
rename to hw/watchdog/wdt_imx2.c
index 2aedfe803a..ad1ef02e9e 100644
--- a/hw/misc/imx2_wdt.c
+++ b/hw/watchdog/wdt_imx2.c
@@ -14,7 +14,7 @@
#include "qemu/module.h"
#include "sysemu/watchdog.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#define IMX2_WDT_WCR_WDABIT(5) /* -> External Reset WDOG_B */
#define IMX2_WDT_WCR_SRSBIT(4) /* -> Software Reset Signal */
diff --git a/include/hw/arm/fsl-imx6.h b/include/hw/arm/fsl-imx6.h
index 60eadccb42..5b02dc1f4b 100644
--- a/include/hw/arm/fsl-imx6.h
+++ b/include/hw/arm/fsl-imx6.h
@@ -21,7 +21,7 @@
#include "hw/cpu/a9mpcore.h"
#include "hw/misc/imx6_ccm.h"
#include "hw/misc/imx6_src.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "hw/char/imx_serial.h"
#include "hw/timer/imx_gpt.h"
#include "hw/timer/imx_epit.h"
diff --git a/include/hw/arm/fsl-imx6ul.h b/include/hw/arm/fsl-imx6ul.h
index eda389aec7..91c746918a 100644
--- a/include/hw/arm/fsl-imx6ul.h
+++ b/include/hw/arm/fsl-imx6ul.h
@@ -24,7 +24,7 @@
#include "hw/misc/imx7_snvs.h"
#include "hw/misc/imx7_gpr.h"
#include "hw/intc/imx_gpcv2.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "hw/gpio/imx_gpio.h"
#include "hw/char/imx_serial.h"
#include "hw/timer/imx_gpt.h"
diff --git a/include/hw/arm/fsl-imx7.h b/include/hw/arm/fsl-imx7.h
index 706aef2e7e..3a0041c4c2 100644
--- a/include/hw/arm/fsl-imx7.h
+++ b/include/hw/arm/fsl-imx7.h
@@ -26,7 +26,7 @@
#include "hw/misc/imx7_snvs.h"
#include "hw/misc/imx7_gpr.h"
#include "hw/misc/imx6_src.h"
-#include "hw/misc/imx2_wdt.h"
+#include "hw/watchdog/wdt_imx2.h"
#include "hw/gpio/imx_gpio.h"
#include "hw/char/imx_serial.h"
#include "hw/timer/imx_gpt.h"
diff --git a/include/hw/misc/imx2_wdt.h b/include/hw/watchdog/wdt_imx2.h
similarity index 100%
rename from include/hw/misc/imx2_wdt.h
rename to include/hw/watchdog/wdt_imx2.h
--
2.17.1
[PATCH 2/8] hw/watchdog: Implement full i.MX watchdog support
Implement full support for the watchdog in i.MX systems.
Pretimeout support is optional because the watchdog hardware on i.MX31
does not support pretimeouts.
Signed-off-by: Guenter Roeck
---
hw/watchdog/Makefile.objs | 2 +-
hw/watchdog/wdt_imx2.c | 196 +++--
include/hw/watchdog/wdt_imx2.h | 49 -
3 files changed, 232 insertions(+), 15 deletions(-)
diff --git a/hw/watchdog/Makefile.objs b/hw/watchdog/Makefile.objs
index dd9b37f642..631b711d86 100644
--- a/hw/watchdog/Makefile.objs
+++ b/hw/watchdog/Makefile.objs
@@ -4,4 +4,4 @@ common-obj-$(CONFIG_WDT_IB6300ESB) += wdt_i6300esb.o
common-obj-$(CONFIG_WDT_IB700) += wdt_ib700.o
common-obj-$(CONFIG_WDT_DIAG288) += wdt_diag288.o
common-obj-$(CONFIG_ASPEED_SOC) += wdt_aspeed.o
-common-obj-$(CONFIG_WDT_IMX) += wdt_imx2.o
+common-obj-$(CONFIG_WDT_IMX2) += wdt_imx2.o
diff --git a/hw/watchdog/wdt_imx2.c b/hw/watchdog/wdt_imx2.c
index ad1ef02e9e..f5339f3590 100644
--- a/hw/watchdog/wdt_imx2.c
+++ b/hw/watchdog/wdt_imx2.c
@@ -13,24 +13,157 @@
#include "qemu/bitops.h"
#include "qemu/module.h"
#include "sysemu/watchdog.h"
+#include "migration/vmstate.h"
+#include "hw/qdev-properties.h"
#include "hw/watchdog/wdt_imx2.h"
-#define IMX2_WDT_WCR_WDABIT(5) /* -> External Reset WDOG_B */
-#define IMX2_WDT_WCR_SRSBIT(4) /* -> Software Reset Signal */
+static void imx2_wdt_interrupt(void *opaque)
+{
+IMX2WdtState *s = IMX2_WDT(opaque);
+
+s->wicr |= IMX2_WDT_WICR_WTIS;
+qemu_set_irq(s->irq, 1);
+}
-static uint64_t imx2_wdt_read(void *opaque, hwaddr addr,
- unsigned int size)
+static void imx2_wdt_expired(void *opaque)
{
+IMX2WdtState *s = IMX2_WDT(opaque);
+
+s->wrsr = IMX2_WDT_WRSR_TOUT;
+
+/* Perform watchdog action if watchdog is enabled */
+if (s->wcr & IMX2_WDT_WCR_WDE) {
+watchdog_perform_action();
+}
+}
+
+static void imx2_wdt_reset(DeviceState *dev)
+{
+IMX2WdtState *s = IMX2_WDT(dev);
+
+s->wcr = IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS;
+s->wsr = 0;
+s->wrsr &= ~(IMX2_WDT_WRSR_TOUT | IMX2_WDT_WRSR_SFTW);
+s->wicr = 4;
+s->wmcr = IMX2_WDT_WMCR_PDE;
+}
+
+static uint64_t imx2_wdt_read(void *opaque, hwaddr addr, unsigned int size)
+{
+IMX2WdtState *s = IMX2_WDT(opaque);
+
+switch (addr) {
+case IMX2_WDT_WCR:
+return s->wcr;
+case IMX2_WDT_WSR:
+return s->wsr;
+case IMX2_WDT_WRSR:
+return s->wrsr;
+case IMX2_WDT_WICR:
+return s->wicr;
+case IMX2_WDT_WMCR:
+return s->wmcr;
+}
return 0;
}
+static void imx_wdt2_update_itimer(IMX2WdtState *s, bool start)
+{
+bool running = (s->wcr & IMX2_WDT_WCR_WDE) && (s->wcr & IMX2_WDT_WCR_WT);
+bool enabled = s->wicr & IMX2_WDT_WICR_WIE;
+
+ptimer_transaction_begin(s->itimer);
+if (start || !enabled) {
+ptimer_stop(s->itimer);
+}
+if (running && enabled) {
+int count = ptimer_get_count(s->timer);
+int pretimeout = s->wicr & IMX2_WDT_WICR_WICT;
+
+/*
+ * Only (re-)start pretimeout timer if its counter value is larger
+ * than 0. Otherwise it will fire right away and we'll get an
+ * interrupt loop.
+ */
+if (count > pretimeout) {
+ptimer_set_count(s->itimer, count - pretimeout);
+if (start) {
+ptimer_run(s->itimer, 1);
+}
+}
+}
+ptimer_transaction_commit(s->itimer);
+}
+
+static void imx_wdt2_update_timer(IMX2WdtState *s, bool start)
+{
+ptimer_transaction_begin(s->timer);
+if (start) {
+ptimer_stop(s->timer);
+}
+if ((s->wcr & IMX2_WDT_WCR_WDE) && (s->wcr & IMX2_WDT_WCR_WT)) {
+int count = (s->wcr & IMX2_WDT_WCR_WT) >> 8;
+
+ptimer_set_count(s->timer, count);
+if (start) {
+ptimer_run(s->timer, 1);
+}
+}
+ptimer_transaction_commit(s->timer);
+if (s->pretimeout_support) {
+imx_wdt2_update_itimer(s, start);
+}
+}
+
static void imx2_wdt_write(void *opaque, hwaddr addr,
uint64_t value, unsigned int size)
{
-if (addr == IMX2_WDT_WCR &&
-(~value & (IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS))) {
-watchdog_perform_action();
+IMX2WdtState *s = IMX2_WDT(opaque);
+
+switch (addr) {
+case IMX2_WDT_WCR:
+s->wcr = value;
+if (!(value & IMX2_WDT_WCR_SRS)) {
+s->wrsr = IMX2_WDT_WRSR_SFTW;
+}
+if (!(value & (IMX2_WDT_WCR_WDA | IMX2_WDT_WCR_SRS)) ||
+(!(value & IMX2_WDT_WCR_WT) && (value & IMX2_WDT_WCR_WDE))) {
+watchdog_perform_action();
+}
+s->wcr |= IMX2_WDT_WCR_SRS;
+imx_wdt2_update_timer(s, true);
+break;
+case IMX2_WDT_WSR:
+if (s->wsr == IMX2_WDT_SEQ1 && value == IMX2_WDT_SEQ2) {
+imx_wdt2_update_timer(s, false);
+}
+s->wsr = value;
+break;
+
Re: [PATCH] linux-user: Update TASK_UNMAPPED_BASE for aarch64
On Sat, Mar 14, 2020 at 11:45 AM Laurent Vivier wrote: > > Le 14/03/2020 à 04:06, Aleksandar Markovic a écrit : > > On Fri, Mar 13, 2020 at 1:28 AM Lirong Yuan wrote: > >> > >> This change updates TASK_UNMAPPED_BASE (the base address for guest > >> programs) for aarch64. It is needed to allow qemu to work with Thread > >> Sanitizer (TSan), which has specific boundary definitions for memory > >> mappings on different platforms: > >> https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/tsan/rtl/tsan_platform.h > >> > >> Signed-off-by: Lirong Yuan > >> --- > >> linux-user/mmap.c | 4 > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c > >> index 8685f02e7e..e378033797 100644 > >> --- a/linux-user/mmap.c > >> +++ b/linux-user/mmap.c > >> @@ -184,7 +184,11 @@ static int mmap_frag(abi_ulong real_start, > >> } > >> > >> #if HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64 > >> +#ifdef TARGET_AARCH64 > >> +# define TASK_UNMAPPED_BASE 0x55 > > > > Hi, Lirong, > > > > Can you point from which line of the file you linked to did you > > arrive to the value 0x55? > > > > Second question: What about other targets? > > Personally, I prefer to not change the value for other targets if it is > not required by someone that had some problems with the current value. > > It needs to be changed carefully and to be well tested after change. > Sure, but again, from where " 0x55" comes from? > Thanks, > Laurent
Re: [PATCH v1] mips/mips_malta: Allow more than 2G RAM
On Sat, Mar 14, 2020 at 1:28 PM Jiaxun Yang wrote: > > > > 在 星期六, 2020-03-14 17:09:08 Philippe Mathieu-Daudé > 撰写 > > Hi Aleksandar, > > > > > >> > > >> This is annoying, because the CoreLV/CoreFPGA core cards only have 4 > > >> DIMM slots for PC-100 SDRAM, and the Memory Controller of the GT–64120A > > >> north bridge accept at most 256 MiB per SCS for address decoding, so we > > >> have a maximum of 1 GiB on 32-bit boards. > > >> > > >> In 64-bit emulation we use the a 20Kc core, provided by the Core20K core > > >> card which doesn't use the GT–64120A but the Bonito64. So the model is > > >> incorrect... The Bonito64 indeed allow more memory. > > >> > > >> Maybe it is time to consider that for 64-bit targets, since we are not > > >> modelling a Malta core board, we can name it the official 'virt' > machine... > > >> > > > > > > Philippe, I almost agree 100% with you wrt last three paragraphs. > > > (in any case, I know much less than you about details of GT-64120A > > > and Bonito64, but I trust you). > > > > > > The only area I have a slightly different opinion is that I believe we > > > should never declare Malta as a virtual board, and try to be within the > > > boundaries of physical capabilities of real boards, even if in software > > > (QEMU) we could "enhance" some features. > > > > > > If we want MIPS virtual board, than we should devise a full blown > > > virtual board, similar to what was already done for MIPS Android > > > emulator. I really don't want some real/virtual frankenstein in QEMU > > > upstream just because it is convenient for let's say a particular > > > test setup. > > So probably it's time to work on a "virt" machine. What style would you > prefer? > PC-like or SoC style? > It is time, agreed. Let's explore multiple alternatives, analyze pros and cons, and not rush into conclusions. There are several important factors to take into account: presence of kernel and QEMU support for involved devices, target users, other virtual machines in QEMU, relation to the general QEMU architecture, available time resources, etc. > In fact we've got two pseudo board (mips, mipssim) for MIPS, > but non of them seems modern enough. > They are terribly outdated, true. > I can launch a Loongson-3 style board after dealing with Kernel code clean-up. > Absolutely! You would be welcomed. Yours, Aleksandar > > > > IIUC today all distributions supporting MIPS ports are building their > > MIPS packages on QEMU instances because it is faster than the native > > MIPS hardware they have. > > > > Since one (or two?) years, some binaries (Linux kernel? QEMU?) are > > failing to link because the amount of guest memory is restricted to 2GB > > (probably advance of linker techniques, now linkers use more memory). > > > > YunQiang, is this why you suggested this change? > > > > See: > > - https://www.mail-archive.com/debian-mips@lists.debian.org/msg10912.html > > - > > > https://alioth-lists.debian.net/pipermail/pkg-rust-maintainers/2019-January/004844.html > > > > I believe most of the QEMU Malta board users don't care it is a Malta > > board, they only care it is a fast emulated MIPS machine. > > Unfortunately it is the default board. > > Yeah. That's the truth. > > > > > However 32-bit MIPS port is being dropped on Debian: > > https://lists.debian.org/debian-mips/2019/07/msg00010.html > > mipsel (MIPS 32-bit Little Endian) is still alive. I believe Debian won't > give up it as long as i386 > still exist. > > > > > Maybe we can sync with the Malta users, ask them to switch to the Boston > > machines to build 64-bit packages, then later reduce the Malta board to > 1GB. > > (The Boston board is more recent, but was not available at the time > > users started to use QEMU to build 64-bit packages). > > > > Might it be easier starting introducing a malta-5.0 machine restricted > > to 1GB? > > > > > > > > Aleksandar > > > > > >>>exit(1); > > >>>} > > >>> +#endif > > >>> > > >>>/* register RAM at high address where it is undisturbed by IO */ > > >>>memory_region_add_subregion(system_memory, 0x8000, > machine->ram); > > >>> > > >> > > >> > > > > > > > > > > -- > Jiaxun Yang >
Re: [PATCH 2/5] tests/docker: make "buildah bud" output similar to "docker build"
Cleber Rosa writes: > Podman users will most often be using buildah to build containers. > Among the differences between "buildah bud|build-using-dockerfile" and > a traditional "docker build" is that buildah does not run a container > during build. > > To the best of my knowledge and experiments, this means that runtime > variables, such as ENV from one base image will not propagate into > another. The end result is that the location for the cross compiler > binaries, defined in the base "qemu/debian9-mxe" image, are not passed > through this image. Consequently, the cross compilers are not on PATH > and the build fails. > > Signed-off-by: Cleber Rosa Acked-by: Alex Bennée > --- > tests/docker/dockerfiles/debian-win32-cross.docker | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tests/docker/dockerfiles/debian-win32-cross.docker > b/tests/docker/dockerfiles/debian-win32-cross.docker > index 9d7053e59d..d16d6431bc 100644 > --- a/tests/docker/dockerfiles/debian-win32-cross.docker > +++ b/tests/docker/dockerfiles/debian-win32-cross.docker > @@ -9,7 +9,7 @@ MAINTAINER Philippe Mathieu-Daudé > > ENV TARGET i686 > > -ENV PATH $PATH:/usr/lib/mxe/usr/$TARGET-w64-mingw32.shared/bin > +ENV PATH > $PATH:/usr/lib/mxe/usr/bin:/usr/lib/mxe/usr/$TARGET-w64-mingw32.shared/bin > > ENV PKG_CONFIG_PATH \ > > $PKG_CONFIG_PATH:/usr/lib/mxe/usr/$TARGET-w64-mingw32.shared/lib/pkgconfig -- Alex Bennée
Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via
On 14/03/20 14:19, Mark Cave-Ayland wrote: >> Observe that mac_via_init() has obvious side effects. In particular, it >> creates two devices that are then visible in "info qtree", and that's >> caught by device-introspect-test. >> >> I believe these things need to be done in .realize(). That is not a problem; the devices should be removed when the device is finalized. In theory the steps would be: - the child properties are removed - this causes unparent to be called on the child devices - this causes the child devices to be unrealized - this causes the child devices to remove themselves from their bus (and from "info qtree") - this causes the refcount to drop to zero and the devices to be finalized themselves. The question is why they are not, i.e. where does the above reasoning break. So, sysbus_init_child_obj is fine. Paolo
Re: [PATCH] tools/virtiofsd: add support for --socket-group
Hi
On Thu, Mar 12, 2020 at 11:49 AM Daniel P. Berrangé wrote:
>
> On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote:
> > If you like running QEMU as a normal user (very common for TCG runs)
> > but you have to run virtiofsd as a root user you run into connection
> > problems. Adding support for an optional --socket-group allows the
> > users to keep using the command line.
>
> If we're going to support this, then I think we need to put it in
> the vhost-user.rst specification so we standardize across backends.
>
>
Perhaps. Otoh, I wonder if the backend spec should be more limited to
arguments/introspection that are used by programs.
In this case, I even consider --socket-path to be unnecessary, as a
management layer can/should provide a preopened & setup fd directly.
What do you think?
>
> > Signed-off-by: Alex Bennée
>
>
> >
> > ---
> > v1
> > - tweak documentation and commentary
> > ---
> > docs/tools/virtiofsd.rst| 4
> > tools/virtiofsd/fuse_i.h| 1 +
> > tools/virtiofsd/fuse_lowlevel.c | 6 ++
> > tools/virtiofsd/fuse_virtio.c | 20 ++--
> > 4 files changed, 29 insertions(+), 2 deletions(-)
> >
> > diff --git a/docs/tools/virtiofsd.rst b/docs/tools/virtiofsd.rst
> > index 378594c422a..5a8246b74f8 100644
> > --- a/docs/tools/virtiofsd.rst
> > +++ b/docs/tools/virtiofsd.rst
> > @@ -85,6 +85,10 @@ Options
> >
> >Listen on vhost-user UNIX domain socket at PATH.
> >
> > +.. option:: --socket-group=GROUP
> > +
> > + Set the vhost-user UNIX domain socket gid to GROUP.
> > +
> > .. option:: --fd=FDNUM
> >
> >Accept connections from vhost-user UNIX domain socket file descriptor
> > FDNUM.
> > diff --git a/tools/virtiofsd/fuse_i.h b/tools/virtiofsd/fuse_i.h
> > index 1240828208a..492e002181e 100644
> > --- a/tools/virtiofsd/fuse_i.h
> > +++ b/tools/virtiofsd/fuse_i.h
> > @@ -68,6 +68,7 @@ struct fuse_session {
> > size_t bufsize;
> > int error;
> > char *vu_socket_path;
> > +char *vu_socket_group;
> > int vu_listen_fd;
> > int vu_socketfd;
> > struct fv_VuDev *virtio_dev;
> > diff --git a/tools/virtiofsd/fuse_lowlevel.c
> > b/tools/virtiofsd/fuse_lowlevel.c
> > index 2dd36ec03b6..4d1ba2925d1 100644
> > --- a/tools/virtiofsd/fuse_lowlevel.c
> > +++ b/tools/virtiofsd/fuse_lowlevel.c
> > @@ -2523,6 +2523,7 @@ static const struct fuse_opt fuse_ll_opts[] = {
> > LL_OPTION("--debug", debug, 1),
> > LL_OPTION("allow_root", deny_others, 1),
> > LL_OPTION("--socket-path=%s", vu_socket_path, 0),
> > +LL_OPTION("--socket-group=%s", vu_socket_group, 0),
> > LL_OPTION("--fd=%d", vu_listen_fd, 0),
> > LL_OPTION("--thread-pool-size=%d", thread_pool_size, 0),
> > FUSE_OPT_END
> > @@ -2630,6 +2631,11 @@ struct fuse_session *fuse_session_new(struct
> > fuse_args *args,
> > "fuse: --socket-path and --fd cannot be given
> > together\n");
> > goto out4;
> > }
> > +if (se->vu_socket_group && !se->vu_socket_path) {
> > +fuse_log(FUSE_LOG_ERR,
> > + "fuse: --socket-group can only be used with
> > --socket-path\n");
> > +goto out4;
> > +}
> >
> > se->bufsize = FUSE_MAX_MAX_PAGES * getpagesize() +
> > FUSE_BUFFER_HEADER_SIZE;
> >
> > diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> > index 3b6d16a0417..331f9fc65c5 100644
> > --- a/tools/virtiofsd/fuse_virtio.c
> > +++ b/tools/virtiofsd/fuse_virtio.c
> > @@ -31,6 +31,8 @@
> > #include
> > #include
> > #include
> > +#include
> > +#include
> > #include
> >
> > #include "contrib/libvhost-user/libvhost-user.h"
> > @@ -924,15 +926,29 @@ static int fv_create_listen_socket(struct
> > fuse_session *se)
> >
> > /*
> > * Unfortunately bind doesn't let you set the mask on the socket,
> > - * so set umask to 077 and restore it later.
> > + * so set umask appropriately and restore it later.
> > */
> > -old_umask = umask(0077);
> > +if (se->vu_socket_group) {
> > +old_umask = umask(S_IROTH | S_IWOTH | S_IXOTH);
> > +} else {
> > +old_umask = umask(S_IRGRP | S_IWGRP | S_IXGRP | S_IROTH | S_IWOTH
> > | S_IXOTH);
> > +}
> > if (bind(listen_sock, (struct sockaddr *), addr_len) == -1) {
> > fuse_log(FUSE_LOG_ERR, "vhost socket bind: %m\n");
> > close(listen_sock);
> > umask(old_umask);
> > return -1;
> > }
> > +if (se->vu_socket_group) {
> > +struct group *g = getgrnam(se->vu_socket_group);
> > +if (g) {
> > +if (!chown(se->vu_socket_path, -1, g->gr_gid)) {
> > +fuse_log(FUSE_LOG_WARNING,
> > + "vhost socket failed to set group to %s (%d)\n",
> > + se->vu_socket_group, g->gr_gid);
> > +}
> > +}
> > +}
> > umask(old_umask);
> >
> > if (listen(listen_sock, 1) == -1) {
> > --
> > 2.20.1
> >
> >
>
> Regards,
>
Re: [PATCH] tools/virtiofsd: add support for --socket-group
On Thu, Mar 12, 2020 at 10:41:42AM +, Alex Bennée wrote: > If you like running QEMU as a normal user (very common for TCG runs) > but you have to run virtiofsd as a root user you run into connection > problems. Adding support for an optional --socket-group allows the > users to keep using the command line. > > Signed-off-by: Alex Bennée > > --- > v1 > - tweak documentation and commentary > --- > docs/tools/virtiofsd.rst| 4 > tools/virtiofsd/fuse_i.h| 1 + > tools/virtiofsd/fuse_lowlevel.c | 6 ++ > tools/virtiofsd/fuse_virtio.c | 20 ++-- > 4 files changed, 29 insertions(+), 2 deletions(-) Dan's suggestion sounds like a good idea to me. Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [PATCH v5 0/4] delay timer_new from init to realize to fix memleaks.
On 14/03/2020 08:47, Pan Nengyuan wrote: > This series delay timer_new from init into realize to avoid memleaks when we > call 'device_list_properties'. > And do timer_free only in s390x_cpu_finalize because it's hotplugable. > However, mos6522_realize is never called > at all due to the incorrect creation of it. So we fix the incorrect creation > in mac_via/cuda/pmu first, then > move the timer_new to mos6522_realize(). > > v1: >- Delay timer_new() from init() to realize() to fix memleaks. > v2: >- Similarly to other cleanups, move timer_new into realize in > target/s390x/cpu.c (Suggested by Philippe Mathieu-Daudé). >- Send these two patches as a series instead of send each as a single > patch but with wrong subject in v1. > v3: >- It's not valid in mos6522 if we move timer_new from init to realize, > because it's never called at all. > Thus, we remove null check in reset, and add calls to mos6522_realize() > in mac_via_realize to make this move to be valid. >- split patch by device to make it more clear. > v4: >- Also do timer_free on the error path in realize() and fix some coding > style. Then use device_class_set_parent_unrealize to declare unrealize. >- split the mos6522 patch into two, one to fix incorrect creation of > mos6522, the other to fix memleak. > > v5: >- Fix two other places where we create mos6522's subclasses but forgot to > realize it(macio/cuda,macio/pmu). > Otherwise, this will cause SEGVs during make check-qtest-ppc64. >- Remove timer_del on the error path of s390x_cpu_realize() and simply use > errp instead a temporary variable. > > Pan Nengyuan (4): > s390x: fix memleaks in cpu_finalize > mac_via: fix incorrect creation of mos6522 device in mac_via > hw/misc/macio: fix incorrect creation of mos6522's subclasses > hw/misc/mos6522: move timer_new from init() into realize() to avoid > memleaks > > hw/misc/mac_via.c | 40 +++- > hw/misc/macio/cuda.c | 11 +-- > hw/misc/macio/pmu.c| 11 +-- > hw/misc/mos6522.c | 6 ++ > target/s390x/cpu-qom.h | 1 + > target/s390x/cpu.c | 30 ++ > 6 files changed, 78 insertions(+), 21 deletions(-) I just gave this a test on qemu-system-ppc -M mac99 with both cuda and pmu, and also qemu-system-m68k for mac_via and I didn't see any crashes there, so: Tested-by: Mark Cave-Ayland ATB, Mark.
Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via
On 10/03/2020 09:07, Markus Armbruster wrote:
> Widespread QOM usage anti-pattern ahead; cc: QOM maintainers.
>
> Peter Maydell writes:
>
>> On Mon, 9 Mar 2020 at 10:02, Pan Nengyuan wrote:
>>> On 3/9/2020 5:21 PM, Peter Maydell wrote:
Could you explain more? My thought is that we should be using
sysbus_init_child_obj() and we should be doing it in the init method.
Why does that break the tests ? It's the same thing various other
devices do.
>>>
>>> device-introspect-test do the follow check for each device type:
>>>
>>> qtree_start = qtest_hmp(qts, "info qtree");
>>> ...
>>> qtest_qmp(qts, "{'execute': 'device-list-properties','arguments':
>>> {'typename': %s}}", type);
>>> ...
>>> qtree_end = qtest_hmp(qts, "info qtree");
>>> g_assert_cmpstr(qtree_start, ==, qtree_end);
>>>
>>> If we do qdev_set_parent_bus in init, it will check fail when type =
>>> 'mac_via'.
>>> mac_via_init() is called by q800_init(). But it will not be called in
>>> qtest(-machine none) in the step qtree_start.
>>> And after we call 'device-list-properties', mac_via_init() was called and
>>> set dev parent bus. We can find these
>>> devices in the qtree_end. So it break the test on the assert.
>>
>> Markus, do you know what's happening here? Why is
>> trying to use sysbus_init_child_obj() breaking the
>> device-introspect-test for this particular device,
>> but fine for the other places where we use it?
>> (Maybe we're accidentally leaking a reference to
>> something so the sub-device stays on the sysbus
>> when it should have removed itself when the
>> device was deinited ?)
>
> Two questions: (1) why does it break here, and (2) why doesn't it break
> elsewhere.
>
> First question: why does it break here?
>
> It breaks here because asking for help with "device_add mac_via,help"
> has a side effect visible in "info qtree".
>
>>> Here is the output:
>>>
>>> # Testing device 'mac_via'
> [Uninteresting stuff snipped...]
>
> "info qtree" before asking for help:
>
>>> qtree_start: bus: main-system-bus
>>> type System
>
> "info qtree" after asking for help:
>
>>> qtree_end: bus: main-system-bus
>>> type System
>>> dev: mos6522-q800-via2, id ""
>>> gpio-in "via2-irq" 8
>>> gpio-out "sysbus-irq" 1
>>> frequency = 0 (0x0)
>>> mmio /0010
>>> dev: mos6522-q800-via1, id ""
>>> gpio-in "via1-irq" 8
>>> gpio-out "sysbus-irq" 1
>>> frequency = 0 (0x0)
>>> mmio /0010
>
> How come?
>
> "device_add mac_via,help" shows properties of device "mac_via". It does
> so even though "mac_via" is not available with device_add. Useful
> because "info qtree" shows properties for such devices.
>
> These properties are defined dynamically, either for the instance
> (traditional) or the class. The former typically happens in QOM
> TypeInfo method .instance_init(), the latter in .class_init().
>
> "Typically", because properties can be added elsewhere, too. In
> particular, QOM properties not meant for device_add are often created in
> DeviceClass method .realize(). More on that below.
>
> To make properties created in .instance_init() visible in help, we need
> to create and destroy an instance. This must be free of observable side
> effects.
>
> This has been such a fertile source of crashes that I added
> device-introspect-test:
>
> commit 2d1abb850fd15fd6eb75a92290be5f93b2772ec5
> Author: Markus Armbruster
> Date: Thu Oct 1 10:59:56 2015 +0200
>
> device-introspect-test: New, covering device introspection
>
> The test doesn't check that the output makes any sense, only that QEMU
> survives. Useful since we've had an astounding number of crash bugs
> around there.
>
> In fact, we have a bunch of them right now: a few devices crash or
> hang, and some leave dangling pointers behind. The test skips testing
> the broken parts. The next commits will fix them up, and drop the
> skipping.
>
> Signed-off-by: Markus Armbruster
> Reviewed-by: Eric Blake
> Message-Id: <144368-12182-8-git-send-email-arm...@redhat.com>
>
> Now let's examine device "mac_via". It defines properties both in its
> .class_init() and its .instance_init().
>
> static void mac_via_class_init(ObjectClass *oc, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(oc);
>
> dc->realize = mac_via_realize;
> dc->reset = mac_via_reset;
> dc->vmsd = _mac_via;
> --->device_class_set_props(dc, mac_via_properties);
> }
>
> where
>
> static Property mac_via_properties[] = {
> DEFINE_PROP_DRIVE("drive", MacVIAState, blk),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> And
>
> static void mac_via_init(Object *obj)
> {
> SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> MacVIAState *m = MAC_VIA(obj);
> MOS6522State *ms;
>
> /* MMIO */
> memory_region_init(>mmio, obj,
Re: [PATCH v1] mips/mips_malta: Allow more than 2G RAM
在 星期六, 2020-03-14 17:09:08 Philippe Mathieu-Daudé 撰写 > Hi Aleksandar, > > >> > >> This is annoying, because the CoreLV/CoreFPGA core cards only have 4 > >> DIMM slots for PC-100 SDRAM, and the Memory Controller of the GT–64120A > >> north bridge accept at most 256 MiB per SCS for address decoding, so we > >> have a maximum of 1 GiB on 32-bit boards. > >> > >> In 64-bit emulation we use the a 20Kc core, provided by the Core20K core > >> card which doesn't use the GT–64120A but the Bonito64. So the model is > >> incorrect... The Bonito64 indeed allow more memory. > >> > >> Maybe it is time to consider that for 64-bit targets, since we are not > >> modelling a Malta core board, we can name it the official 'virt' > >> machine... > >> > > > > Philippe, I almost agree 100% with you wrt last three paragraphs. > > (in any case, I know much less than you about details of GT-64120A > > and Bonito64, but I trust you). > > > > The only area I have a slightly different opinion is that I believe we > > should never declare Malta as a virtual board, and try to be within the > > boundaries of physical capabilities of real boards, even if in software > > (QEMU) we could "enhance" some features. > > > > If we want MIPS virtual board, than we should devise a full blown > > virtual board, similar to what was already done for MIPS Android > > emulator. I really don't want some real/virtual frankenstein in QEMU > > upstream just because it is convenient for let's say a particular > > test setup. So probably it's time to work on a "virt" machine. What style would you prefer? PC-like or SoC style? In fact we've got two pseudo board (mips, mipssim) for MIPS, but non of them seems modern enough. I can launch a Loongson-3 style board after dealing with Kernel code clean-up. > > IIUC today all distributions supporting MIPS ports are building their > MIPS packages on QEMU instances because it is faster than the native > MIPS hardware they have. > > Since one (or two?) years, some binaries (Linux kernel? QEMU?) are > failing to link because the amount of guest memory is restricted to 2GB > (probably advance of linker techniques, now linkers use more memory). > > YunQiang, is this why you suggested this change? > > See: > - https://www.mail-archive.com/debian-mips@lists.debian.org/msg10912.html > - > https://alioth-lists.debian.net/pipermail/pkg-rust-maintainers/2019-January/004844.html > > I believe most of the QEMU Malta board users don't care it is a Malta > board, they only care it is a fast emulated MIPS machine. > Unfortunately it is the default board. Yeah. That's the truth. > > However 32-bit MIPS port is being dropped on Debian: > https://lists.debian.org/debian-mips/2019/07/msg00010.html mipsel (MIPS 32-bit Little Endian) is still alive. I believe Debian won't give up it as long as i386 still exist. > > Maybe we can sync with the Malta users, ask them to switch to the Boston > machines to build 64-bit packages, then later reduce the Malta board to 1GB. > (The Boston board is more recent, but was not available at the time > users started to use QEMU to build 64-bit packages). > > Might it be easier starting introducing a malta-5.0 machine restricted > to 1GB? > > > > > Aleksandar > > > >>>exit(1); > >>>} > >>> +#endif > >>> > >>>/* register RAM at high address where it is undisturbed by IO */ > >>>memory_region_add_subregion(system_memory, 0x8000, > >>> machine->ram); > >>> > >> > >> > > > > -- Jiaxun Yang
Re: [PATCH v2 0/4] linux-user: generate syscall_nr.h from linux unistd.h
Patchew URL: https://patchew.org/QEMU/20200314113922.233353-1-laur...@vivier.eu/
Hi,
This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.
=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==6502==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-coroutine"
==6557==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
==6557==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top:
0x7ffe190d3000; bottom 0x7f4e297aa000; size: 0x00afef929000 (755638636544)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==6572==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img
tests/qtest/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="ide-test"
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-aio-multithread -m=quick -k --tap < /dev/null |
./scripts/tap-driver.pl --test-name="test-aio-multithread"
==6580==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
==6586==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
PASS 1 ide-test /x86_64/ide/identify
==6600==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 2 ide-test /x86_64/ide/flush
==6611==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==6617==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 ide-test /x86_64/ide/bmdma/trim
==6628==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-throttle"
==6645==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
--test-name="test-thread-pool"
==6649==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==6716==WARNING: ASan doesn't fully support makecontext/swapcontext functions
and may produce false positives in some cases!
PASS 5 test-thread-pool /thread-pool/cancel
PASS 6 test-thread-pool /thread-pool/cancel-async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}
tests/test-hbitmap -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl
Re: [PATCH v3 07/16] hw/i386/vmport: Introduce vmport.h
On 14/03/2020 10:31, Philippe Mathieu-Daudé wrote: On 3/13/20 11:38 PM, Liran Alon wrote: On 13/03/2020 21:57, Philippe Mathieu-Daudé wrote: On 3/12/20 5:54 PM, Liran Alon wrote: No functional change. This is mere refactoring. Suggested-by: Michael S. Tsirkin Signed-off-by: Liran Alon --- hw/i386/pc.c | 1 + hw/i386/vmmouse.c | 1 + hw/i386/vmport.c | 1 + include/hw/i386/pc.h | 13 - include/hw/i386/vmport.h | 16 What about moving it to hw/i386/vmport.h (no under include/)? Reviewed-by: Philippe Mathieu-Daudé Can you explain the logic that separates between hw/i386/*.h to include/hw/i386/*.h? Headers in the include/hw/ namespace can be consumed by all machine targets. But this doesn't seem true for headers in include/hw/i386/*.h... It contains things that are target-specific. E.g. ioapic.h, x86-iommu.h, intel_iommu.h and etc. I still don't quite understand the separation between these directories. It seems both are i386-specific and one of them shouldn't exists. If this is a target-specific device, having it local to the target (hw/i386/) protect generic code (and other targets) of using it. This helps detecting wrong dependencies between components. If it makes sense, sure I will move it. I just don't know what is the convention here. Michael/Paolo/Eduardo what do you recommend?
Re: [PATCH 0/8] Misc hw/ide legacy clean up
On 14/03/20 12:45, Mark Cave-Ayland wrote: > On 13/03/2020 21:14, BALATON Zoltan wrote: > >> These are some clean ups to remove more legacy init functions and >> lessen dependence on include/hw/ide.h with some simplifications in >> board code. There should be no functional change. >> >> BALATON Zoltan (8): >> hw/ide: Get rid of piix3_init functions >> hw/ide: Get rid of piix4_init function >> hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h >> hw/ide: Move MAX_IDE_BUS define to one header >> hw/ide/pci.c: Coding style update to fix checkpatch errors >> hw/ide: Do ide_drive_get() within pci_ide_create_devs() >> hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h >> hw/ide: Remove unneeded inclusion of hw/ide.h >> >> hw/alpha/dp264.c | 15 +++ >> hw/hppa/hppa_sys.h| 1 - >> hw/hppa/machine.c | 3 --- >> hw/i386/pc_piix.c | 20 +--- >> hw/ide/ahci_internal.h| 1 + >> hw/ide/pci.c | 10 ++ >> hw/ide/piix.c | 31 +-- >> hw/isa/piix4.c| 14 +- >> hw/mips/mips_fulong2e.c | 6 +- >> hw/mips/mips_malta.c | 6 ++ >> hw/mips/mips_r4k.c| 4 +--- >> hw/ppc/mac_newworld.c | 2 -- >> hw/ppc/mac_oldworld.c | 2 -- >> hw/ppc/prep.c | 3 --- >> hw/sparc64/sun4u.c| 7 +-- >> include/hw/ide.h | 6 -- >> include/hw/ide/internal.h | 3 +++ >> include/hw/ide/pci.h | 3 ++- >> include/hw/misc/macio/macio.h | 1 + >> include/hw/southbridge/piix.h | 3 +-- >> 20 files changed, 37 insertions(+), 104 deletions(-) > > This looks like a good clean-up to me, but certainly it would be good to get > a second > opinion from people more familiar with the IDE code internals. > > Reviewed-by: Mark Cave-Ayland Yes, it looks good to me. Thanks! Paolo
Re: [PATCH 0/8] Misc hw/ide legacy clean up
On 13/03/2020 21:14, BALATON Zoltan wrote: > These are some clean ups to remove more legacy init functions and > lessen dependence on include/hw/ide.h with some simplifications in > board code. There should be no functional change. > > BALATON Zoltan (8): > hw/ide: Get rid of piix3_init functions > hw/ide: Get rid of piix4_init function > hw/ide: Remove now unneded #include "hw/pci/pci.h" from hw/ide.h > hw/ide: Move MAX_IDE_BUS define to one header > hw/ide/pci.c: Coding style update to fix checkpatch errors > hw/ide: Do ide_drive_get() within pci_ide_create_devs() > hw/ide: Move MAX_IDE_DEVS define to hw/ide/internal.h > hw/ide: Remove unneeded inclusion of hw/ide.h > > hw/alpha/dp264.c | 15 +++ > hw/hppa/hppa_sys.h| 1 - > hw/hppa/machine.c | 3 --- > hw/i386/pc_piix.c | 20 +--- > hw/ide/ahci_internal.h| 1 + > hw/ide/pci.c | 10 ++ > hw/ide/piix.c | 31 +-- > hw/isa/piix4.c| 14 +- > hw/mips/mips_fulong2e.c | 6 +- > hw/mips/mips_malta.c | 6 ++ > hw/mips/mips_r4k.c| 4 +--- > hw/ppc/mac_newworld.c | 2 -- > hw/ppc/mac_oldworld.c | 2 -- > hw/ppc/prep.c | 3 --- > hw/sparc64/sun4u.c| 7 +-- > include/hw/ide.h | 6 -- > include/hw/ide/internal.h | 3 +++ > include/hw/ide/pci.h | 3 ++- > include/hw/misc/macio/macio.h | 1 + > include/hw/southbridge/piix.h | 3 +-- > 20 files changed, 37 insertions(+), 104 deletions(-) This looks like a good clean-up to me, but certainly it would be good to get a second opinion from people more familiar with the IDE code internals. Reviewed-by: Mark Cave-Ayland ATB, Mark.
[PATCH v2 4/4] linux-user, openrisc: sync syscall numbers with kernel v5.5
Use helper script scripts/gensyscalls.sh to generate the file. Add TARGET_NR_or1k_atomic Remove useless comments and blank lines. Define diretly the __NR_XXX64 syscalls rather than using the intermediate __NR3264 definition. Remove wrong cut'n'paste (like "#ifdef __ARCH_WANT_SYNC_FILE_RANGE2") Add new syscalls from 286 (preadv) to 434 (pidfd_open). Remove obsolete syscalls 1204 (open) to 1079 (fork). Signed-off-by: Laurent Vivier Reviewed-by: Alistair Francis --- Notes: v2: add comments suggested by Taylor linux-user/openrisc/syscall_nr.h | 309 +++ 1 file changed, 62 insertions(+), 247 deletions(-) diff --git a/linux-user/openrisc/syscall_nr.h b/linux-user/openrisc/syscall_nr.h index 7763dbcfd8b3..340383beb2c6 100644 --- a/linux-user/openrisc/syscall_nr.h +++ b/linux-user/openrisc/syscall_nr.h @@ -1,13 +1,17 @@ +/* + * This file contains the system call numbers. + * Do not modify. + * This file is generated by scripts/gensyscalls.sh + */ #ifndef LINUX_USER_OPENRISC_SYSCALL_NR_H #define LINUX_USER_OPENRISC_SYSCALL_NR_H #define TARGET_NR_io_setup 0 +#define TARGET_NR_or1k_atomic TARGET_NR_arch_specific_syscall #define TARGET_NR_io_destroy 1 #define TARGET_NR_io_submit 2 #define TARGET_NR_io_cancel 3 #define TARGET_NR_io_getevents 4 - -/* fs/xattr.c */ #define TARGET_NR_setxattr 5 #define TARGET_NR_lsetxattr 6 #define TARGET_NR_fsetxattr 7 @@ -20,63 +24,36 @@ #define TARGET_NR_removexattr 14 #define TARGET_NR_lremovexattr 15 #define TARGET_NR_fremovexattr 16 - -/* fs/dcache.c */ #define TARGET_NR_getcwd 17 - -/* fs/cookies.c */ #define TARGET_NR_lookup_dcookie 18 - -/* fs/eventfd.c */ #define TARGET_NR_eventfd2 19 - -/* fs/eventpoll.c */ #define TARGET_NR_epoll_create1 20 #define TARGET_NR_epoll_ctl 21 #define TARGET_NR_epoll_pwait 22 - -/* fs/fcntl.c */ #define TARGET_NR_dup 23 #define TARGET_NR_dup3 24 -#define TARGET_NR_3264_fcntl 25 - -/* fs/inotify_user.c */ +#define TARGET_NR_fcntl64 25 #define TARGET_NR_inotify_init1 26 #define TARGET_NR_inotify_add_watch 27 #define TARGET_NR_inotify_rm_watch 28 - -/* fs/ioctl.c */ #define TARGET_NR_ioctl 29 - -/* fs/ioprio.c */ #define TARGET_NR_ioprio_set 30 #define TARGET_NR_ioprio_get 31 - -/* fs/locks.c */ #define TARGET_NR_flock 32 - -/* fs/namei.c */ #define TARGET_NR_mknodat 33 #define TARGET_NR_mkdirat 34 #define TARGET_NR_unlinkat 35 #define TARGET_NR_symlinkat 36 #define TARGET_NR_linkat 37 #define TARGET_NR_renameat 38 - -/* fs/namespace.c */ #define TARGET_NR_umount2 39 #define TARGET_NR_mount 40 #define TARGET_NR_pivot_root 41 - -/* fs/nfsctl.c */ #define TARGET_NR_nfsservctl 42 - -/* fs/open.c */ -#define TARGET_NR_3264_statfs 43 -#define TARGET_NR_3264_fstatfs 44 -#define TARGET_NR_3264_truncate 45 -#define TARGET_NR_3264_ftruncate 46 - +#define TARGET_NR_statfs64 43 +#define TARGET_NR_fstatfs64 44 +#define TARGET_NR_truncate64 45 +#define TARGET_NR_ftruncate64 46 #define TARGET_NR_fallocate 47 #define TARGET_NR_faccessat 48 #define TARGET_NR_chdir 49 @@ -89,18 +66,10 @@ #define TARGET_NR_openat 56 #define TARGET_NR_close 57 #define TARGET_NR_vhangup 58 - -/* fs/pipe.c */ #define TARGET_NR_pipe2 59 - -/* fs/quota.c */ #define TARGET_NR_quotactl 60 - -/* fs/readdir.c */ #define TARGET_NR_getdents64 61 - -/* fs/read_write.c */ -#define TARGET_NR_3264_lseek 62 +#define TARGET_NR_llseek 62 #define TARGET_NR_read 63 #define TARGET_NR_write 64 #define TARGET_NR_readv 65 @@ -109,85 +78,42 @@ #define TARGET_NR_pwrite64 68 #define TARGET_NR_preadv 69 #define TARGET_NR_pwritev 70 - -/* fs/sendfile.c */ -#define TARGET_NR_3264_sendfile 71 - -/* fs/select.c */ +#define TARGET_NR_sendfile64 71 #define TARGET_NR_pselect6 72 #define TARGET_NR_ppoll 73 - -/* fs/signalfd.c */ #define TARGET_NR_signalfd4 74 - -/* fs/splice.c */ #define TARGET_NR_vmsplice 75 #define TARGET_NR_splice 76 #define TARGET_NR_tee 77 - -/* fs/stat.c */ #define TARGET_NR_readlinkat 78 -#define TARGET_NR_3264_fstatat 79 -#define TARGET_NR_3264_fstat 80 - -/* fs/sync.c */ +#define TARGET_NR_fstatat64 79 +#define TARGET_NR_fstat64 80 #define TARGET_NR_sync 81 #define TARGET_NR_fsync 82 #define TARGET_NR_fdatasync 83 - -#ifdef __ARCH_WANT_SYNC_FILE_RANGE2 -#define TARGET_NR_sync_file_range2 84 -#else #define TARGET_NR_sync_file_range 84 -#endif - -/* fs/timerfd.c */ #define TARGET_NR_timerfd_create 85 #define TARGET_NR_timerfd_settime 86 #define TARGET_NR_timerfd_gettime 87 - -/* fs/utimes.c */ #define TARGET_NR_utimensat 88 - -/* kernel/acct.c */ #define TARGET_NR_acct 89 - -/* kernel/capability.c */ #define TARGET_NR_capget 90 #define TARGET_NR_capset 91 - -/* kernel/exec_domain.c */ #define TARGET_NR_personality 92 - -/* kernel/exit.c */ #define TARGET_NR_exit 93 #define TARGET_NR_exit_group 94 #define TARGET_NR_waitid 95 - -/* kernel/fork.c */ #define TARGET_NR_set_tid_address 96 #define TARGET_NR_unshare 97 - -/* kernel/futex.c */ #define TARGET_NR_futex 98 #define
[PATCH v2 0/4] linux-user: generate syscall_nr.h from linux unistd.h
This series adds a script to generate syscall_nr.h for architectures that don't use syscall.tbl but asm-generic/unistd.h The script uses several cpp passes and filters result with a grep/sed/tr sequence. The result must be checked before being used, so it's why the script is not automatically run. I have run the script, checked and added new files for arm64, nios2, openrisc. I don't include result for riscv as Alistair is already working on a series for this architecture and it needs some changes in syscall.c as some syscalls are not defined. We also need to add the _time64 variant of syscalls added by the update of the syscall_nr.h. Based-on: <20200310103403.3284090-1-laur...@vivier.eu> v2: add comments suggested by Taylor Laurent Vivier (4): scripts: add a script to generate syscall_nr.h linux-user, aarch64: sync syscall numbers with kernel v5.5 linux-user,nios2: sync syscall numbers with kernel v5.5 linux-user, openrisc: sync syscall numbers with kernel v5.5 linux-user/aarch64/syscall_nr.h | 34 +- linux-user/nios2/syscall_nr.h| 650 +++ linux-user/openrisc/syscall_nr.h | 309 +++ scripts/gensyscalls.sh | 102 + 4 files changed, 513 insertions(+), 582 deletions(-) create mode 100755 scripts/gensyscalls.sh -- 2.24.1
[PATCH v2 2/4] linux-user, aarch64: sync syscall numbers with kernel v5.5
Use helper script scripts/gensyscalls.sh to generate the file. This change TARGET_NR_fstatat64 by TARGET_NR_newfstatat that is correct because definitions from linux are: arch/arm64/include/uapi/asm/unistd.h #define __ARCH_WANT_NEW_STAT include/uapi/asm-generic/unistd.h #if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64) #define __NR3264_fstatat 79 __SC_3264(__NR3264_fstatat, sys_fstatat64, sys_newfstatat) #define __NR3264_fstat 80 __SC_3264(__NR3264_fstat, sys_fstat64, sys_newfstat) #endif ... #if __BITS_PER_LONG == 64 && !defined(__SYSCALL_COMPAT) ... #if defined(__ARCH_WANT_NEW_STAT) || defined(__ARCH_WANT_STAT64) #define __NR_newfstatat __NR3264_fstatat #define __NR_fstat __NR3264_fstat #endif ... Add syscalls 286 (preadv2) to 435 (clone3). Signed-off-by: Laurent Vivier Reviewed-by: Alistair Francis --- Notes: v2: add comments suggested by Taylor linux-user/aarch64/syscall_nr.h | 34 - 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/linux-user/aarch64/syscall_nr.h b/linux-user/aarch64/syscall_nr.h index f00ffd7fb82f..85de000b2490 100644 --- a/linux-user/aarch64/syscall_nr.h +++ b/linux-user/aarch64/syscall_nr.h @@ -1,7 +1,8 @@ /* * This file contains the system call numbers. + * Do not modify. + * This file is generated by scripts/gensyscalls.sh */ - #ifndef LINUX_USER_AARCH64_SYSCALL_NR_H #define LINUX_USER_AARCH64_SYSCALL_NR_H @@ -84,7 +85,7 @@ #define TARGET_NR_splice 76 #define TARGET_NR_tee 77 #define TARGET_NR_readlinkat 78 -#define TARGET_NR_fstatat64 79 +#define TARGET_NR_newfstatat 79 #define TARGET_NR_fstat 80 #define TARGET_NR_sync 81 #define TARGET_NR_fsync 82 @@ -254,8 +255,8 @@ #define TARGET_NR_prlimit64 261 #define TARGET_NR_fanotify_init 262 #define TARGET_NR_fanotify_mark 263 -#define TARGET_NR_name_to_handle_at 264 -#define TARGET_NR_open_by_handle_at 265 +#define TARGET_NR_name_to_handle_at 264 +#define TARGET_NR_open_by_handle_at 265 #define TARGET_NR_clock_adjtime 266 #define TARGET_NR_syncfs 267 #define TARGET_NR_setns 268 @@ -276,5 +277,28 @@ #define TARGET_NR_membarrier 283 #define TARGET_NR_mlock2 284 #define TARGET_NR_copy_file_range 285 +#define TARGET_NR_preadv2 286 +#define TARGET_NR_pwritev2 287 +#define TARGET_NR_pkey_mprotect 288 +#define TARGET_NR_pkey_alloc 289 +#define TARGET_NR_pkey_free 290 +#define TARGET_NR_statx 291 +#define TARGET_NR_io_pgetevents 292 +#define TARGET_NR_rseq 293 +#define TARGET_NR_kexec_file_load 294 +#define TARGET_NR_pidfd_send_signal 424 +#define TARGET_NR_io_uring_setup 425 +#define TARGET_NR_io_uring_enter 426 +#define TARGET_NR_io_uring_register 427 +#define TARGET_NR_open_tree 428 +#define TARGET_NR_move_mount 429 +#define TARGET_NR_fsopen 430 +#define TARGET_NR_fsconfig 431 +#define TARGET_NR_fsmount 432 +#define TARGET_NR_fspick 433 +#define TARGET_NR_pidfd_open 434 +#define TARGET_NR_clone3 435 +#define TARGET_NR_syscalls 436 + +#endif /* LINUX_USER_AARCH64_SYSCALL_NR_H */ -#endif -- 2.24.1
[PATCH v2 3/4] linux-user, nios2: sync syscall numbers with kernel v5.5
Use helper script scripts/gensyscalls.sh to generate the file. This adds TARGET_NR_llseek that was missing and remove syscalls 1024 to 1079. Add new syscalls from 288 (pkey_mprotect) to 434 (pidfd_open) Signed-off-by: Laurent Vivier Reviewed-by: Alistair Francis --- Notes: v2: add comments suggested by Taylor linux-user/nios2/syscall_nr.h | 650 +- 1 file changed, 320 insertions(+), 330 deletions(-) diff --git a/linux-user/nios2/syscall_nr.h b/linux-user/nios2/syscall_nr.h index 8fb87864ca0b..32d485dc9ae8 100644 --- a/linux-user/nios2/syscall_nr.h +++ b/linux-user/nios2/syscall_nr.h @@ -1,334 +1,324 @@ +/* + * This file contains the system call numbers. + * Do not modify. + * This file is generated by scripts/gensyscalls.sh + */ #ifndef LINUX_USER_NIOS2_SYSCALL_NR_H #define LINUX_USER_NIOS2_SYSCALL_NR_H -#define TARGET_NR_io_setup 0 -#define TARGET_NR_io_destroy1 -#define TARGET_NR_io_submit 2 -#define TARGET_NR_io_cancel 3 -#define TARGET_NR_io_getevents 4 -#define TARGET_NR_setxattr 5 -#define TARGET_NR_lsetxattr 6 -#define TARGET_NR_fsetxattr 7 -#define TARGET_NR_getxattr 8 -#define TARGET_NR_lgetxattr 9 -#define TARGET_NR_fgetxattr 10 -#define TARGET_NR_listxattr 11 -#define TARGET_NR_llistxattr12 -#define TARGET_NR_flistxattr13 -#define TARGET_NR_removexattr 14 -#define TARGET_NR_lremovexattr 15 -#define TARGET_NR_fremovexattr 16 -#define TARGET_NR_getcwd17 -#define TARGET_NR_lookup_dcookie18 -#define TARGET_NR_eventfd2 19 -#define TARGET_NR_epoll_create1 20 -#define TARGET_NR_epoll_ctl 21 -#define TARGET_NR_epoll_pwait 22 -#define TARGET_NR_dup 23 -#define TARGET_NR_dup3 24 -#define TARGET_NR_fcntl64 25 -#define TARGET_NR_inotify_init1 26 -#define TARGET_NR_inotify_add_watch 27 -#define TARGET_NR_inotify_rm_watch 28 -#define TARGET_NR_ioctl 29 -#define TARGET_NR_ioprio_set30 -#define TARGET_NR_ioprio_get31 -#define TARGET_NR_flock 32 -#define TARGET_NR_mknodat 33 -#define TARGET_NR_mkdirat 34 -#define TARGET_NR_unlinkat 35 -#define TARGET_NR_symlinkat 36 -#define TARGET_NR_linkat37 -#define TARGET_NR_renameat 38 -#define TARGET_NR_umount2 39 -#define TARGET_NR_mount 40 -#define TARGET_NR_pivot_root41 -#define TARGET_NR_nfsservctl42 -#define TARGET_NR_statfs64 43 -#define TARGET_NR_fstatfs64 44 -#define TARGET_NR_truncate6445 -#define TARGET_NR_ftruncate64 46 -#define TARGET_NR_fallocate 47 -#define TARGET_NR_faccessat 48 -#define TARGET_NR_chdir 49 -#define TARGET_NR_fchdir50 -#define TARGET_NR_chroot51 -#define TARGET_NR_fchmod52 -#define TARGET_NR_fchmodat 53 -#define TARGET_NR_fchownat 54 -#define TARGET_NR_fchown55 -#define TARGET_NR_openat56 -#define TARGET_NR_close 57 -#define TARGET_NR_vhangup 58 -#define TARGET_NR_pipe2 59 -#define TARGET_NR_quotactl 60 -#define TARGET_NR_getdents6461 -#define TARGET_NR_read 63 -#define TARGET_NR_write 64 -#define TARGET_NR_readv 65 -#define TARGET_NR_writev66 -#define TARGET_NR_pread64 67 -#define TARGET_NR_pwrite64 68 -#define TARGET_NR_preadv69 -#define TARGET_NR_pwritev 70 -#define TARGET_NR_sendfile6471 -#define TARGET_NR_pselect6 72 -#define TARGET_NR_ppoll 73 -#define TARGET_NR_signalfd4 74 -#define TARGET_NR_vmsplice 75 -#define TARGET_NR_splice76 -#define TARGET_NR_tee 77 -#define TARGET_NR_readlinkat78 -#define TARGET_NR_fstatat64 79 -#define TARGET_NR_fstat64 80 -#define TARGET_NR_sync 81 -#define TARGET_NR_fsync 82 -#define TARGET_NR_fdatasync 83 -#define TARGET_NR_sync_file_range 84 -#define TARGET_NR_timerfd_create85 -#define TARGET_NR_timerfd_settime 86 -#define
[PATCH v2 1/4] scripts: add a script to generate syscall_nr.h
This script is needed for targets based on asm-generic syscall numbers
generation
Signed-off-by: Laurent Vivier
Reviewed-by: Alistair Francis
Reviewed-by: Taylor Simpson
---
Notes:
v2: add comments suggested by Taylor
scripts/gensyscalls.sh | 102 +
1 file changed, 102 insertions(+)
create mode 100755 scripts/gensyscalls.sh
diff --git a/scripts/gensyscalls.sh b/scripts/gensyscalls.sh
new file mode 100755
index ..71557ab45be3
--- /dev/null
+++ b/scripts/gensyscalls.sh
@@ -0,0 +1,102 @@
+#!/bin/sh
+#
+# Update syscall_nr.h files from linux headers asm-generic/unistd.h
+#
+# This code is licensed under the GPL version 2 or later. See
+# the COPYING file in the top-level directory.
+#
+
+linux="$1"
+output="$2"
+
+TMP=$(mktemp -d)
+
+if [ "$linux" = "" ] ; then
+echo "Needs path to linux source tree" 1>&2
+exit 1
+fi
+
+if [ "$output" = "" ] ; then
+output="$PWD"
+fi
+
+upper()
+{
+echo "$1" | tr "[:lower:]" "[:upper:]" | tr "[:punct:]" "_"
+}
+
+qemu_arch()
+{
+case "$1" in
+arm64)
+echo "aarch64"
+;;
+*)
+upper "$1"
+;;
+esac
+}
+
+read_includes()
+{
+arch=$1
+bits=$2
+
+ cpp -P -nostdinc -fdirectives-only \
+-D_UAPI_ASM_$(upper ${arch})_BITSPERLONG_H \
+-D__BITS_PER_LONG=${bits} \
+-I${linux}/arch/${arch}/include/uapi/ \
+-I${linux}/include/uapi \
+-I${TMP} \
+"${linux}/arch/${arch}/include/uapi/asm/unistd.h"
+}
+
+filter_defines()
+{
+grep -e "#define __NR_" -e "#define __NR3264"
+}
+
+rename_defines()
+{
+sed "s/ __NR_/ TARGET_NR_/g;s/(__NR_/(TARGET_NR_/g"
+}
+
+evaluate_values()
+{
+sed "s/#define TARGET_NR_/QEMU TARGET_NR_/" | \
+cpp -P -nostdinc | \
+sed "s/^QEMU /#define /"
+}
+
+generate_syscall_nr()
+{
+arch=$1
+bits=$2
+file="$3"
+guard="$(upper LINUX_USER_$(qemu_arch $arch)_$(basename "$file"))"
+
+(echo "/*"
+echo " * This file contains the system call numbers."
+echo " * Do not modify."
+echo " * This file is generated by scripts/gensyscalls.sh"
+echo " */"
+echo "#ifndef ${guard}"
+echo "#define ${guard}"
+echo
+read_includes $arch $bits | filter_defines | rename_defines | \
+evaluate_values | sort -n -k 3
+echo
+echo "#endif /* ${guard} */"
+echo) > "$file"
+}
+
+mkdir "$TMP/asm"
+> "$TMP/asm/bitsperlong.h"
+
+generate_syscall_nr arm64 64 "$output/linux-user/aarch64/syscall_nr.h"
+generate_syscall_nr nios2 32 "$output/linux-user/nios2/syscall_nr.h"
+generate_syscall_nr openrisc 32 "$output/linux-user/openrisc/syscall_nr.h"
+
+generate_syscall_nr riscv 32 "$output/linux-user/riscv/syscall32_nr.h"
+generate_syscall_nr riscv 64 "$output/linux-user/riscv/syscall64_nr.h"
+rm -fr "$TMP"
--
2.24.1
Re: [PATCH] softmmu/vl.c: Handle '-cpu help' and '-device help' before 'no default machine'
Peter Maydell writes: > Currently if you try to ask for the list of CPUs for a target > architecture which does not specify a default machine type > you just get an error: > > $ qemu-system-arm -cpu help > qemu-system-arm: No machine specified, and there is no default > Use -machine help to list supported machines > > Since the list of CPUs doesn't depend on the machine, this is > unnecessarily unhelpful. "-device help" has a similar problem. > > Move the checks for "did the user ask for -cpu help or -device help" > up so they precede the select_machine() call which checks that the > user specified a valid machine type. > > Signed-off-by: Peter Maydell > --- > This has been on-and-off irritating me for years, and it's > embarrassing how simple the fix turns out to be... Same here. The patch works as advertized, thus: Reviewed-by: Markus Armbruster Can you offer a completeness argument? We call is_help_option() and qemu_opt_has_help_opt() from quite a few places.
Re: [PATCH v8 3/4] linux-user: Support futex_time64
Le 14/03/2020 à 00:56, Alistair Francis a écrit :
> Add support for host and target futex_time64. If futex_time64 exists on
> the host we try that first before falling back to the standard futux
> syscall.
>
> Signed-off-by: Alistair Francis
> ---
> linux-user/syscall.c | 144 +++
> 1 file changed, 131 insertions(+), 13 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 60fd775d9c..b3bfb02688 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -245,7 +245,12 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4
> arg4,type5 arg5,\
> #define __NR_sys_rt_sigqueueinfo __NR_rt_sigqueueinfo
> #define __NR_sys_rt_tgsigqueueinfo __NR_rt_tgsigqueueinfo
> #define __NR_sys_syslog __NR_syslog
> -#define __NR_sys_futex __NR_futex
> +#if defined(__NR_futex)
> +# define __NR_sys_futex __NR_futex
> +#endif
> +#if defined(__NR_futex_time64)
> +# define __NR_sys_futex_time64 __NR_futex_time64
> +#endif
> #define __NR_sys_inotify_init __NR_inotify_init
> #define __NR_sys_inotify_add_watch __NR_inotify_add_watch
> #define __NR_sys_inotify_rm_watch __NR_inotify_rm_watch
> @@ -295,10 +300,16 @@ _syscall1(int,exit_group,int,error_code)
> #if defined(TARGET_NR_set_tid_address) && defined(__NR_set_tid_address)
> _syscall1(int,set_tid_address,int *,tidptr)
> #endif
> -#if defined(TARGET_NR_futex) && defined(__NR_futex)
> +#if (defined(TARGET_NR_futex) && defined(__NR_futex)) || \
> +(defined(TARGET_NR_futex_time64) && \
> +(HOST_LONG_BITS == 64 && defined(__NR_futex)))
> _syscall6(int,sys_futex,int *,uaddr,int,op,int,val,
>const struct timespec *,timeout,int *,uaddr2,int,val3)
> #endif
> +#if (defined(TARGET_NR_futex_time64) && defined(__NR_futex_teim64))
> +_syscall6(int,sys_futex_time64,int *,uaddr,int,op,int,val,
> + const struct timespec *,timeout,int *,uaddr2,int,val3)
> +#endif
> #define __NR_sys_sched_getaffinity __NR_sched_getaffinity
> _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned int, len,
>unsigned long *, user_mask_ptr);
> @@ -762,10 +773,14 @@ safe_syscall5(int, ppoll, struct pollfd *, ufds,
> unsigned int, nfds,
> safe_syscall6(int, epoll_pwait, int, epfd, struct epoll_event *, events,
>int, maxevents, int, timeout, const sigset_t *, sigmask,
>size_t, sigsetsize)
> -#ifdef TARGET_NR_futex
> +#if defined(__NR_futex)
> safe_syscall6(int,futex,int *,uaddr,int,op,int,val, \
>const struct timespec *,timeout,int *,uaddr2,int,val3)
> #endif
> +#if defined(__NR_futex_time64)
> +safe_syscall6(int,futex_time64,int *,uaddr,int,op,int,val, \
> + const struct timespec *,timeout,int *,uaddr2,int,val3)
> +#endif
> safe_syscall2(int, rt_sigsuspend, sigset_t *, newset, size_t, sigsetsize)
> safe_syscall2(int, kill, pid_t, pid, int, sig)
> safe_syscall2(int, tkill, int, tid, int, sig)
> @@ -1229,7 +1244,7 @@ static inline abi_long target_to_host_timespec(struct
> timespec *host_ts,
> }
> #endif
>
> -#if defined(TARGET_NR_clock_settime64)
> +#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64)
> static inline abi_long target_to_host_timespec64(struct timespec *host_ts,
> abi_ulong target_addr)
> {
> @@ -6890,6 +6905,55 @@ static inline abi_long host_to_target_statx(struct
> target_statx *host_stx,
> }
> #endif
>
> +static int do_sys_futex(int *uaddr, int op, int val,
> + const struct timespec *timeout, int *uaddr2,
> + int val3)
> +{
> +#if HOST_LONG_BITS == 64
> +#if defined(__NR_futex)
> +/* always a 64-bit time_t, it doesn't define _time64 version */
> +return sys_futex(uaddr, op, val, timeout, uaddr2, val3);
> +
> +#endif
> +#else /* HOST_LONG_BITS == 64 */
> +#if defined(__NR_futex_time64)
> +if (sizeof(timeout->tv_sec) == 8) {
> +/* _time64 function on 32bit arch */
> +return sys_futex_time64(uaddr, op, val, timeout, uaddr2, val3);
> +}
> +#endif
> +#if defined(__NR_futex)
> +/* old function on 32bit arch */
> +return sys_futex(uaddr, op, val, timeout, uaddr2, val3);
> +#endif
> +#endif /* HOST_LONG_BITS == 64 */
> +g_assert_not_reached();
> +}
> +
> +static int do_safe_futex(int *uaddr, int op, int val,
> + const struct timespec *timeout, int *uaddr2,
> + int val3)
> +{
> +#if HOST_LONG_BITS == 64
> +#if defined(__NR_futex)
> +/* always a 64-bit time_t, it doesn't define _time64 version */
> +return get_errno(safe_futex(uaddr, op, val, timeout, uaddr2, val3));
> +#endif
> +#else /* HOST_LONG_BITS == 64 */
> +#if defined(__NR_futex_time64)
> +if (sizeof(timeout->tv_sec) == 8) {
> +/* _time64 function on 32bit arch */
> +return get_errno(safe_futex_time64(uaddr, op, val, timeout, uaddr2,
> +
Re: [PATCH v8 3/4] linux-user: Support futex_time64
Le 14/03/2020 à 00:56, Alistair Francis a écrit :
> Add support for host and target futex_time64. If futex_time64 exists on
> the host we try that first before falling back to the standard futux
> syscall.
>
> Signed-off-by: Alistair Francis
> ---
> linux-user/syscall.c | 144 +++
> 1 file changed, 131 insertions(+), 13 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 60fd775d9c..b3bfb02688 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -245,7 +245,12 @@ static type name (type1 arg1,type2 arg2,type3 arg3,type4
> arg4,type5 arg5,\
> #define __NR_sys_rt_sigqueueinfo __NR_rt_sigqueueinfo
> #define __NR_sys_rt_tgsigqueueinfo __NR_rt_tgsigqueueinfo
> #define __NR_sys_syslog __NR_syslog
> -#define __NR_sys_futex __NR_futex
> +#if defined(__NR_futex)
> +# define __NR_sys_futex __NR_futex
> +#endif
> +#if defined(__NR_futex_time64)
> +# define __NR_sys_futex_time64 __NR_futex_time64
> +#endif
> #define __NR_sys_inotify_init __NR_inotify_init
> #define __NR_sys_inotify_add_watch __NR_inotify_add_watch
> #define __NR_sys_inotify_rm_watch __NR_inotify_rm_watch
> @@ -295,10 +300,16 @@ _syscall1(int,exit_group,int,error_code)
> #if defined(TARGET_NR_set_tid_address) && defined(__NR_set_tid_address)
> _syscall1(int,set_tid_address,int *,tidptr)
> #endif
> -#if defined(TARGET_NR_futex) && defined(__NR_futex)
> +#if (defined(TARGET_NR_futex) && defined(__NR_futex)) || \
> +(defined(TARGET_NR_futex_time64) && \
> +(HOST_LONG_BITS == 64 && defined(__NR_futex)))
> _syscall6(int,sys_futex,int *,uaddr,int,op,int,val,
>const struct timespec *,timeout,int *,uaddr2,int,val3)
> #endif
> +#if (defined(TARGET_NR_futex_time64) && defined(__NR_futex_teim64))
> +_syscall6(int,sys_futex_time64,int *,uaddr,int,op,int,val,
> + const struct timespec *,timeout,int *,uaddr2,int,val3)
> +#endif
> #define __NR_sys_sched_getaffinity __NR_sched_getaffinity
> _syscall3(int, sys_sched_getaffinity, pid_t, pid, unsigned int, len,
>unsigned long *, user_mask_ptr);
> @@ -762,10 +773,14 @@ safe_syscall5(int, ppoll, struct pollfd *, ufds,
> unsigned int, nfds,
> safe_syscall6(int, epoll_pwait, int, epfd, struct epoll_event *, events,
>int, maxevents, int, timeout, const sigset_t *, sigmask,
>size_t, sigsetsize)
> -#ifdef TARGET_NR_futex
> +#if defined(__NR_futex)
> safe_syscall6(int,futex,int *,uaddr,int,op,int,val, \
>const struct timespec *,timeout,int *,uaddr2,int,val3)
> #endif
> +#if defined(__NR_futex_time64)
> +safe_syscall6(int,futex_time64,int *,uaddr,int,op,int,val, \
> + const struct timespec *,timeout,int *,uaddr2,int,val3)
> +#endif
> safe_syscall2(int, rt_sigsuspend, sigset_t *, newset, size_t, sigsetsize)
> safe_syscall2(int, kill, pid_t, pid, int, sig)
> safe_syscall2(int, tkill, int, tid, int, sig)
> @@ -1229,7 +1244,7 @@ static inline abi_long target_to_host_timespec(struct
> timespec *host_ts,
> }
> #endif
>
> -#if defined(TARGET_NR_clock_settime64)
> +#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64)
> static inline abi_long target_to_host_timespec64(struct timespec *host_ts,
> abi_ulong target_addr)
> {
> @@ -6890,6 +6905,55 @@ static inline abi_long host_to_target_statx(struct
> target_statx *host_stx,
> }
> #endif
>
> +static int do_sys_futex(int *uaddr, int op, int val,
> + const struct timespec *timeout, int *uaddr2,
> + int val3)
> +{
> +#if HOST_LONG_BITS == 64
> +#if defined(__NR_futex)
> +/* always a 64-bit time_t, it doesn't define _time64 version */
> +return sys_futex(uaddr, op, val, timeout, uaddr2, val3);
> +
> +#endif
> +#else /* HOST_LONG_BITS == 64 */
> +#if defined(__NR_futex_time64)
> +if (sizeof(timeout->tv_sec) == 8) {
> +/* _time64 function on 32bit arch */
> +return sys_futex_time64(uaddr, op, val, timeout, uaddr2, val3);
> +}
> +#endif
> +#if defined(__NR_futex)
> +/* old function on 32bit arch */
> +return sys_futex(uaddr, op, val, timeout, uaddr2, val3);
> +#endif
> +#endif /* HOST_LONG_BITS == 64 */
> +g_assert_not_reached();
> +}
> +
> +static int do_safe_futex(int *uaddr, int op, int val,
> + const struct timespec *timeout, int *uaddr2,
> + int val3)
> +{
> +#if HOST_LONG_BITS == 64
> +#if defined(__NR_futex)
> +/* always a 64-bit time_t, it doesn't define _time64 version */
> +return get_errno(safe_futex(uaddr, op, val, timeout, uaddr2, val3));
> +#endif
> +#else /* HOST_LONG_BITS == 64 */
> +#if defined(__NR_futex_time64)
> +if (sizeof(timeout->tv_sec) == 8) {
> +/* _time64 function on 32bit arch */
> +return get_errno(safe_futex_time64(uaddr, op, val, timeout, uaddr2,
> +
Re: [PATCH 8/9] hw/core: Add qdev stub for user-mode
On 14/03/20 11:49, Philippe Mathieu-Daudé wrote: >>> >>> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs >>> index 6215e7c208..89bf247173 100644 >>> --- a/hw/core/Makefile.objs >>> +++ b/hw/core/Makefile.objs >>> @@ -8,6 +8,7 @@ common-obj-y += vmstate-if.o >>> # irq.o needed for qdev GPIO handling: >>> common-obj-y += irq.o >>> +common-obj-$(call lnot,$(CONFIG_SOFTMMU)) += qdev-stubs.o >> >> This should be: >> >> obj-$(call lnot,$(CONFIG_SOFTMMU)) += qdev-stubs.o > > Actually I moved it to stub-obj-y which makes things easier. No, common-obj- is the right thing, followed by common-obj-$(CONFIG_ALL) += qdev-stubs.o Paolo
Re: [PATCH 8/9] hw/core: Add qdev stub for user-mode
On 3/14/20 10:46 AM, Philippe Mathieu-Daudé wrote:
On 3/13/20 7:46 PM, Philippe Mathieu-Daudé wrote:
While user-mode does not use peripherals (devices), it uses a
CPU which is a device.
In the next commit we will reduce the QAPI generated code for
user-mode. Since qdev.c calls qapi_event_send_device_deleted(),
let's add a stub for it.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/core/qdev-stubs.c | 20
hw/core/Makefile.objs | 1 +
2 files changed, 21 insertions(+)
create mode 100644 hw/core/qdev-stubs.c
diff --git a/hw/core/qdev-stubs.c b/hw/core/qdev-stubs.c
new file mode 100644
index 00..0819dcba12
--- /dev/null
+++ b/hw/core/qdev-stubs.c
@@ -0,0 +1,20 @@
+/*
+ * QAPI qdev stubs
+ *
+ * Copyright (c) 2020 Red Hat, Inc.
+ *
+ * Author:
+ * Philippe Mathieu-Daudé
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-events-qdev.h"
+
+void qapi_event_send_device_deleted(bool has_device,
+ const char *device, const char
*path)
+{
+}
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 6215e7c208..89bf247173 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -8,6 +8,7 @@ common-obj-y += vmstate-if.o
# irq.o needed for qdev GPIO handling:
common-obj-y += irq.o
+common-obj-$(call lnot,$(CONFIG_SOFTMMU)) += qdev-stubs.o
This should be:
obj-$(call lnot,$(CONFIG_SOFTMMU)) += qdev-stubs.o
Actually I moved it to stub-obj-y which makes things easier.
(not common).
common-obj-$(CONFIG_SOFTMMU) += reset.o
common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
Re: [PATCH] linux-user: Update TASK_UNMAPPED_BASE for aarch64
Le 14/03/2020 à 04:06, Aleksandar Markovic a écrit : > On Fri, Mar 13, 2020 at 1:28 AM Lirong Yuan wrote: >> >> This change updates TASK_UNMAPPED_BASE (the base address for guest programs) >> for aarch64. It is needed to allow qemu to work with Thread Sanitizer >> (TSan), which has specific boundary definitions for memory mappings on >> different platforms: >> https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/tsan/rtl/tsan_platform.h >> >> Signed-off-by: Lirong Yuan >> --- >> linux-user/mmap.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/linux-user/mmap.c b/linux-user/mmap.c >> index 8685f02e7e..e378033797 100644 >> --- a/linux-user/mmap.c >> +++ b/linux-user/mmap.c >> @@ -184,7 +184,11 @@ static int mmap_frag(abi_ulong real_start, >> } >> >> #if HOST_LONG_BITS == 64 && TARGET_ABI_BITS == 64 >> +#ifdef TARGET_AARCH64 >> +# define TASK_UNMAPPED_BASE 0x55 > > Hi, Lirong, > > Can you point from which line of the file you linked to did you > arrive to the value 0x55? > > Second question: What about other targets? Personally, I prefer to not change the value for other targets if it is not required by someone that had some problems with the current value. It needs to be changed carefully and to be well tested after change. Thanks, Laurent
Re: [PATCH 6/8] hw/ide: Do ide_drive_get() within pci_ide_create_devs()
On 13/03/20 23:16, BALATON Zoltan wrote: >> >> + pci_dev = pci_create_simple(pci_bus, -1, "cmd646-ide"); >> + pci_ide_create_devs(pci_dev); > > Additionally, I think it may also make sense to move pci_ide_create_devs > call into the realize methods of these IDE controllers so boards do not > need to do it explicitely. These calls always follow the creation of the > device immediately so could just be done internally in IDE device and > simplify it further. I can attempt to prepare additional patches for > that but first I'd like to hear if anyone has anything against that to > avoid doing useless work. No, it's better to do it separately. I think that otherwise you could add another IDE controller with -device, and both controllers would try to add the drives. Basically, separating the call means that only automatically added controllers obey "if=ide". Paolo
Re: [PATCH v2] exec/rom_reset: Free rom data during inmigrate skip
On 13/03/20 17:02, Peter Maydell wrote: > Reviewed-by: Peter Maydell Queued, thanks. Paolo
Re: [PATCH 8/9] hw/core: Add qdev stub for user-mode
On 3/13/20 7:46 PM, Philippe Mathieu-Daudé wrote:
While user-mode does not use peripherals (devices), it uses a
CPU which is a device.
In the next commit we will reduce the QAPI generated code for
user-mode. Since qdev.c calls qapi_event_send_device_deleted(),
let's add a stub for it.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/core/qdev-stubs.c | 20
hw/core/Makefile.objs | 1 +
2 files changed, 21 insertions(+)
create mode 100644 hw/core/qdev-stubs.c
diff --git a/hw/core/qdev-stubs.c b/hw/core/qdev-stubs.c
new file mode 100644
index 00..0819dcba12
--- /dev/null
+++ b/hw/core/qdev-stubs.c
@@ -0,0 +1,20 @@
+/*
+ * QAPI qdev stubs
+ *
+ * Copyright (c) 2020 Red Hat, Inc.
+ *
+ * Author:
+ * Philippe Mathieu-Daudé
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-events-qdev.h"
+
+void qapi_event_send_device_deleted(bool has_device,
+const char *device, const char *path)
+{
+}
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 6215e7c208..89bf247173 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -8,6 +8,7 @@ common-obj-y += vmstate-if.o
# irq.o needed for qdev GPIO handling:
common-obj-y += irq.o
+common-obj-$(call lnot,$(CONFIG_SOFTMMU)) += qdev-stubs.o
This should be:
obj-$(call lnot,$(CONFIG_SOFTMMU)) += qdev-stubs.o
(not common).
common-obj-$(CONFIG_SOFTMMU) += reset.o
common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
Re: [PATCH v2 0/2] Fix MAP_SYNC support when host has older glibc version
On 12/03/20 00:23, Eduardo Habkost wrote: > Changes v1 -> v2: > * Use -isystem for $PWD/linux-headers too > Reported-by: "Michael S. Tsirkin" > > This is an alternative to the patch submitted at: > > From: Jingqi Liu > Subject: [PATCH] util: fix to get configuration macros in util/mmap-alloc.c > Date: Thu, 5 Mar 2020 23:41:42 +0800 > Message-Id: <20200305154142.63070-1-jingqi@intel.com> > > Before moving the osdep.h include to the top of the file, we had > to address warnings triggered when was included > after (done in patch 1/2). > > Eduardo Habkost (2): > Use -isystem for linux-headers dir > mmap-alloc: Include osdep.h before checking CONFIG_LINUX > > Makefile.target | 2 +- > configure | 2 +- > util/mmap-alloc.c | 7 +++ > 3 files changed, 5 insertions(+), 6 deletions(-) > Queued, thanks. Paolo
Re: [PATCH 0/7] via-ide: fixes and improvements
On 13/03/2020 17:57, John Snow wrote: > On 3/13/20 4:24 AM, Mark Cave-Ayland wrote: >> Following on from the earlier thread "Implement "non 100% native mode" >> in via-ide", here is an updated patchset based upon the test cases >> sent to me off-list. >> >> The VIA IDE controller is similar to early versions of the PIIX >> controller in that the primary and secondary IDE channels are hardwired >> to IRQs 14 and 15 respectively. Guest OSs typically handle this by >> either switching the controller to legacy mode, or using native mode and >> using a combination of PCI device/vendor ID and/or checking various >> registers in PCI configuration space to detect this condition and apply >> a special fixed IRQ 14/15 routing. >> >> This patchset effectively updates the VIA IDE PCI device to follow the >> behaviour in the datasheet in two ways: fixing some PCI configuration >> space register defaults and behaviours, and always using legacy IRQ 14/15 >> routing, and once applied allows all our known test images to boot >> correctly. >> >> Signed-off-by: Mark Cave-Ayland >> >> >> BALATON Zoltan (2): >> ide/via: Get rid of via_ide_init() >> pci: Honour wmask when resetting PCI_INTERRUPT_LINE >> >> Mark Cave-Ayland (5): >> via-ide: move registration of VMStateDescription to DeviceClass >> via-ide: ensure that PCI_INTERRUPT_LINE is hard-wired to its default >> value >> via-ide: initialise IDE controller in legacy mode >> via-ide: allow guests to write to PCI_CLASS_PROG >> via-ide: always use legacy IRQ 14/15 routing >> >> hw/ide/via.c| 21 + >> hw/mips/mips_fulong2e.c | 5 - >> hw/pci/pci.c| 5 - >> include/hw/ide.h| 1 - >> 4 files changed, 13 insertions(+), 19 deletions(-) >> > > Does this supersede everything else so far? (Except the two cmd646 > related series, four patches total, which are already staged) Yes, that's correct. It passes all our tests, and even better allows the fulong2e CD image at the link Zoltan posted to boot. So I believe it's good unless Alexander has any objections? ATB, Mark.
Re: [PATCH v5 39/60] target/riscv: vector floating-point classify instructions
On 2020/3/14 17:10, Richard Henderson wrote:
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
+/* Vector Floating-Point Classify Instruction */
+static uint16_t fclass_f16(uint16_t frs1, float_status *s)
+{
+float16 f = frs1;
+bool sign = float16_is_neg(f);
+
+if (float16_is_infinity(f)) {
+return sign ? 1 << 0 : 1 << 7;
+} else if (float16_is_zero(f)) {
+return sign ? 1 << 3 : 1 << 4;
+} else if (float16_is_zero_or_denormal(f)) {
+return sign ? 1 << 2 : 1 << 5;
+} else if (float16_is_any_nan(f)) {
+float_status s = { }; /* for snan_bit_is_one */
+return float16_is_quiet_nan(f, ) ? 1 << 9 : 1 << 8;
+} else {
+return sign ? 1 << 1 : 1 << 6;
+}
+}
+static uint32_t fclass_s(uint32_t frs1, float_status *s)
+{
+float32 f = frs1;
+bool sign = float32_is_neg(f);
+
+if (float32_is_infinity(f)) {
+return sign ? 1 << 0 : 1 << 7;
+} else if (float32_is_zero(f)) {
+return sign ? 1 << 3 : 1 << 4;
+} else if (float32_is_zero_or_denormal(f)) {
+return sign ? 1 << 2 : 1 << 5;
+} else if (float32_is_any_nan(f)) {
+float_status s = { }; /* for snan_bit_is_one */
+return float32_is_quiet_nan(f, ) ? 1 << 9 : 1 << 8;
+} else {
+return sign ? 1 << 1 : 1 << 6;
+}
+}
+static uint64_t fclass_d(uint64_t frs1, float_status *s)
+{
+float64 f = frs1;
+bool sign = float64_is_neg(f);
+
+if (float64_is_infinity(f)) {
+return sign ? 1 << 0 : 1 << 7;
+} else if (float64_is_zero(f)) {
+return sign ? 1 << 3 : 1 << 4;
+} else if (float64_is_zero_or_denormal(f)) {
+return sign ? 1 << 2 : 1 << 5;
+} else if (float64_is_any_nan(f)) {
+float_status s = { }; /* for snan_bit_is_one */
+return float64_is_quiet_nan(f, ) ? 1 << 9 : 1 << 8;
+} else {
+return sign ? 1 << 1 : 1 << 6;
+}
+}
These need to be moved out of fpu_helper.c so they can be shared.
I will add an internals.h and move the declaration to internals.h.
Zhiwei
r~
Re: [PATCH v5 38/60] target/riscv: vector floating-point compare instructions
On 2020/3/14 17:08, Richard Henderson wrote:
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
+static uint8_t float16_eq_quiet(uint16_t a, uint16_t b, float_status *s)
+{
+int compare = float16_compare_quiet(a, b, s);
+if (compare == float_relation_equal) {
+return 1;
+} else {
+return 0;
+}
+}
You really need remember that boolean results in C are 1 and 0.
You do not need to keep translating true to 1 and false to 0.
Got it. I was not very sure it is 1 or non 0 for true before.
Zhiwei
+static uint8_t vmfne16(uint16_t a, uint16_t b, float_status *s)
+{
+int compare = float16_compare_quiet(a, b, s);
+if (compare != float_relation_equal &&
+compare != float_relation_unordered) {
Indentation.
+static uint8_t float16_le(uint16_t a, uint16_t b, float_status *s)
+{
+int compare = float16_compare(a, b, s);
+if (compare == float_relation_less ||
+compare == float_relation_equal) {
+return 1;
Indentation.
r~
Re: [PATCH v5 39/60] target/riscv: vector floating-point classify instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +/* Vector Floating-Point Classify Instruction */
> +static uint16_t fclass_f16(uint16_t frs1, float_status *s)
> +{
> +float16 f = frs1;
> +bool sign = float16_is_neg(f);
> +
> +if (float16_is_infinity(f)) {
> +return sign ? 1 << 0 : 1 << 7;
> +} else if (float16_is_zero(f)) {
> +return sign ? 1 << 3 : 1 << 4;
> +} else if (float16_is_zero_or_denormal(f)) {
> +return sign ? 1 << 2 : 1 << 5;
> +} else if (float16_is_any_nan(f)) {
> +float_status s = { }; /* for snan_bit_is_one */
> +return float16_is_quiet_nan(f, ) ? 1 << 9 : 1 << 8;
> +} else {
> +return sign ? 1 << 1 : 1 << 6;
> +}
> +}
> +static uint32_t fclass_s(uint32_t frs1, float_status *s)
> +{
> +float32 f = frs1;
> +bool sign = float32_is_neg(f);
> +
> +if (float32_is_infinity(f)) {
> +return sign ? 1 << 0 : 1 << 7;
> +} else if (float32_is_zero(f)) {
> +return sign ? 1 << 3 : 1 << 4;
> +} else if (float32_is_zero_or_denormal(f)) {
> +return sign ? 1 << 2 : 1 << 5;
> +} else if (float32_is_any_nan(f)) {
> +float_status s = { }; /* for snan_bit_is_one */
> +return float32_is_quiet_nan(f, ) ? 1 << 9 : 1 << 8;
> +} else {
> +return sign ? 1 << 1 : 1 << 6;
> +}
> +}
> +static uint64_t fclass_d(uint64_t frs1, float_status *s)
> +{
> +float64 f = frs1;
> +bool sign = float64_is_neg(f);
> +
> +if (float64_is_infinity(f)) {
> +return sign ? 1 << 0 : 1 << 7;
> +} else if (float64_is_zero(f)) {
> +return sign ? 1 << 3 : 1 << 4;
> +} else if (float64_is_zero_or_denormal(f)) {
> +return sign ? 1 << 2 : 1 << 5;
> +} else if (float64_is_any_nan(f)) {
> +float_status s = { }; /* for snan_bit_is_one */
> +return float64_is_quiet_nan(f, ) ? 1 << 9 : 1 << 8;
> +} else {
> +return sign ? 1 << 1 : 1 << 6;
> +}
> +}
These need to be moved out of fpu_helper.c so they can be shared.
r~
Re: [PATCH v1] mips/mips_malta: Allow more than 2G RAM
Hi Aleksandar,
(+Aurelien for Debian)
(+Peter regarding changing default)
On 3/14/20 4:25 AM, Aleksandar Markovic wrote:
On Thu, Mar 5, 2020 at 11:18 AM Philippe Mathieu-Daudé
wrote:
Please post new patches as v2, and do not post them as reply to v1.
On 3/3/20 1:41 AM, Jiaxun Yang wrote:
When malta is coupled with MIPS64 cpu which have 64bit
address space, it is possible to have more than 2G RAM.
So we removed ram_size check and overwrite memory
layout for these targets.
Signed-off-by: Jiaxun Yang
Suggested-by: Yunqiang Su
--
v1: Do not overwrite cmdline when we don't have highmem.
---
hw/mips/mips_malta.c | 29 +++--
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 6e7ba9235d..4267958f35 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -98,7 +98,8 @@ typedef struct {
} MaltaState;
static struct _loaderparams {
-int ram_size, ram_low_size;
+unsigned int ram_low_size;
+ram_addr_t ram_size;
const char *kernel_filename;
const char *kernel_cmdline;
const char *initrd_filename;
@@ -1023,6 +1024,7 @@ static int64_t load_kernel(void)
{
int64_t kernel_entry, kernel_high, initrd_size;
long kernel_size;
+char mem_cmdline[128];
ram_addr_t initrd_offset;
int big_endian;
uint32_t *prom_buf;
@@ -1099,20 +1101,33 @@ static int64_t load_kernel(void)
prom_buf = g_malloc(prom_size);
prom_set(prom_buf, prom_index++, "%s", loaderparams.kernel_filename);
+
+memset(_cmdline[0], 0, sizeof(mem_cmdline));
+if (loaderparams.ram_size > 0x1000) {
Please use 256 * MiB.
+/*
+ * Always use cmdline to overwrite mem layout
+ * as kernel may reject large emesize.
+ */
+sprintf(_cmdline[0],
+"mem=0x1000@0x mem=0x%" PRIx64 "@0x9000",
+loaderparams.ram_size - 0x1000);
Ditto.
Also please use g_strdup_printf("mem=0x%" PRIx64 "@...")/g_free.
+}
+
if (initrd_size > 0) {
prom_set(prom_buf, prom_index++,
- "rd_start=0x%" PRIx64 " rd_size=%" PRId64 " %s",
- xlate_to_kseg0(NULL, initrd_offset),
+ "%s rd_start=0x%" PRIx64 " rd_size=%" PRId64 " %s",
+ _cmdline[0], xlate_to_kseg0(NULL, initrd_offset),
initrd_size, loaderparams.kernel_cmdline);
} else {
-prom_set(prom_buf, prom_index++, "%s", loaderparams.kernel_cmdline);
+prom_set(prom_buf, prom_index++, "%s %s", _cmdline[0],
+ loaderparams.kernel_cmdline);
}
prom_set(prom_buf, prom_index++, "memsize");
prom_set(prom_buf, prom_index++, "%u", loaderparams.ram_low_size);
prom_set(prom_buf, prom_index++, "ememsize");
-prom_set(prom_buf, prom_index++, "%u", loaderparams.ram_size);
+prom_set(prom_buf, prom_index++, "%lu", loaderparams.ram_size);
prom_set(prom_buf, prom_index++, "modetty0");
prom_set(prom_buf, prom_index++, "38400n8r");
@@ -1253,12 +1268,14 @@ void mips_malta_init(MachineState *machine)
/* create CPU */
mips_create_cpu(machine, s, _irq, _irq);
-/* allocate RAM */
+#ifdef TARGET_MIPS32
+/* MIPS32 won't accept more than 2GiB RAM due to limited address space */
Cc'ing Paul Burton due to commit 94c2b6aff43.
if (ram_size > 2 * GiB) {
error_report("Too much memory for this machine: %" PRId64 "MB,"
" maximum 2048MB", ram_size / MiB);
This is annoying, because the CoreLV/CoreFPGA core cards only have 4
DIMM slots for PC-100 SDRAM, and the Memory Controller of the GT–64120A
north bridge accept at most 256 MiB per SCS for address decoding, so we
have a maximum of 1 GiB on 32-bit boards.
In 64-bit emulation we use the a 20Kc core, provided by the Core20K core
card which doesn't use the GT–64120A but the Bonito64. So the model is
incorrect... The Bonito64 indeed allow more memory.
Maybe it is time to consider that for 64-bit targets, since we are not
modelling a Malta core board, we can name it the official 'virt' machine...
Philippe, I almost agree 100% with you wrt last three paragraphs.
(in any case, I know much less than you about details of GT-64120A
and Bonito64, but I trust you).
The only area I have a slightly different opinion is that I believe we
should never declare Malta as a virtual board, and try to be within the
boundaries of physical capabilities of real boards, even if in software
(QEMU) we could "enhance" some features.
If we want MIPS virtual board, than we should devise a full blown
virtual board, similar to what was already done for MIPS Android
emulator. I really don't want some real/virtual frankenstein in QEMU
upstream just because it is convenient for let's say a particular
test setup.
IIUC today all distributions supporting MIPS ports are building their
MIPS packages on QEMU
Re: [PATCH v5 38/60] target/riscv: vector floating-point compare instructions
On 3/12/20 7:58 AM, LIU Zhiwei wrote:
> +static uint8_t float16_eq_quiet(uint16_t a, uint16_t b, float_status *s)
> +{
> +int compare = float16_compare_quiet(a, b, s);
> +if (compare == float_relation_equal) {
> +return 1;
> +} else {
> +return 0;
> +}
> +}
You really need remember that boolean results in C are 1 and 0.
You do not need to keep translating true to 1 and false to 0.
> +static uint8_t vmfne16(uint16_t a, uint16_t b, float_status *s)
> +{
> +int compare = float16_compare_quiet(a, b, s);
> +if (compare != float_relation_equal &&
> +compare != float_relation_unordered) {
Indentation.
> +static uint8_t float16_le(uint16_t a, uint16_t b, float_status *s)
> +{
> +int compare = float16_compare(a, b, s);
> +if (compare == float_relation_less ||
> +compare == float_relation_equal) {
> +return 1;
Indentation.
r~
