[PATCH] Avoid unaligned fetch in ladr_match()
There is no guarantee that the PCNetState is allocated such that
csr[8] is allocated on an 8-byte boundary. Since not all hosts are
capable of unaligned fetches the 16-bit elements need to be fetched
individually to avoid a potential fault. Closes issue #2143
Signed-off-by: Nick Briggs
---
hw/net/pcnet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c
index 494eab8479..ad675ab29d 100644
--- a/hw/net/pcnet.c
+++ b/hw/net/pcnet.c
@@ -632,7 +632,7 @@ static inline int ladr_match(PCNetState *s, const uint8_t
*buf, int size)
{
struct qemu_ether_header *hdr = (void *)buf;
if ((*(hdr->ether_dhost)&0x01) &&
-((uint64_t *)&s->csr[8])[0] != 0LL) {
+(s->csr[8] | s->csr[9] | s->csr[10] | s->csr[11]) != 0) {
uint8_t ladr[8] = {
s->csr[8] & 0xff, s->csr[8] >> 8,
s->csr[9] & 0xff, s->csr[9] >> 8,
--
2.31.1
Re: [PATCH 1/2] migration/rdma: define htonll/ntohll only if not predefined
Thank you. Yes, with those two patches applied I have compiled qemu on Solaris 11.4 running on a SPARC-T4-1 (sun4v) system to emulate a single target: an HP PA-RISC. > On Jan 14, 2024, at 8:35 PM, Peter Xu wrote: > > On Thu, Jan 11, 2024 at 01:20:17PM -0500, Nick Briggs wrote: >> Solaris has #defines for htonll and ntohll which cause syntax errors >> when compiling code that attempts to (re)define these functions.. >> >> Signed-off-by: Nick Briggs > > I left the other QGA patch for QGA maintainers, assuming this will enable > solaries build for qemu itself alone. > > Queued this one for migration-staging. > > Thanks, > > -- > Peter Xu >
[PATCH 2/2] qga: Solaris has net/if_arp.h and netinet/if_ether.h but not ETHER_ADDR_LEN
Solaris has net/if_arp.h and netinet/if_ether.h rather than net/ethernet.h, but does not define ETHER_ADDR_LEN, instead providing ETHERADDRL. Signed-off-by: Nick Briggs --- qga/commands-posix.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 6169bbf7a0..26008db497 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -45,9 +45,12 @@ #include #include #include -#if defined(__NetBSD__) || defined(__OpenBSD__) +#if defined(__NetBSD__) || defined(__OpenBSD__) || defined(CONFIG_SOLARIS) #include #include +#if !defined(ETHER_ADDR_LEN) && defined(ETHERADDRL) +#define ETHER_ADDR_LEN ETHERADDRL +#endif #else #include #endif -- 2.31.1
[PATCH 1/2] migration/rdma: define htonll/ntohll only if not predefined
Solaris has #defines for htonll and ntohll which cause syntax errors
when compiling code that attempts to (re)define these functions..
Signed-off-by: Nick Briggs
---
migration/rdma.c | 4
1 file changed, 4 insertions(+)
diff --git a/migration/rdma.c b/migration/rdma.c
index 94c0f871f0..a355dcea89 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -238,6 +238,7 @@ static const char *control_desc(unsigned int rdma_control)
return strs[rdma_control];
}
+#if !defined(htonll)
static uint64_t htonll(uint64_t v)
{
union { uint32_t lv[2]; uint64_t llv; } u;
@@ -245,13 +246,16 @@ static uint64_t htonll(uint64_t v)
u.lv[1] = htonl(v & 0xULL);
return u.llv;
}
+#endif
+#if !defined(ntohll)
static uint64_t ntohll(uint64_t v)
{
union { uint32_t lv[2]; uint64_t llv; } u;
u.llv = v;
return ((uint64_t)ntohl(u.lv[0]) << 32) | (uint64_t) ntohl(u.lv[1]);
}
+#endif
static void dest_block_to_network(RDMADestBlock *db)
{
--
2.31.1
Re: [PATCH 2/8] target/sparc: Fix VIS fmul8x16au instruction.
On 2023-09-28, Richard Henderson wrote:
> Belated follow-up suggestion:
>
> - if ((tmp & 0xff) > 0x7f) {
> - tmp += 0x100;
> - }
> + tmp += 0x80;
>
> 7 occurrences throughout vis_helper.c.
I agree with making this particular change but I think since it doesn't
fix a bug, it should go in a separate patch.
So I will include a patch to do that in series v2 and keep this one
as-is with your Reviewed-by.
Thanks,
Nick
Re: [PATCH 6/8] target/sparc: Fix VIS fpmerge input registers.
On 2023-09-28, Richard Henderson wrote: > On 9/24/23 01:03, Nick Bowler wrote: >> case 0x04b: /* VIS I fpmerge */ >> CHECK_FPU_FEATURE(dc, VIS1); >> -gen_ne_fop_DDD(dc, rd, rs1, rs2, >> gen_helper_fpmerge); >> +cpu_src1_32 = gen_load_fpr_F(dc, rs1); >> +cpu_src2_32 = gen_load_fpr_F(dc, rs2); >> +cpu_dst_64 = gen_dest_fpr_D(dc, rd); >> +gen_helper_fpmerge(cpu_dst_64, cpu_src1_32, >> cpu_src2_32); >> +gen_store_fpr_D(dc, rd, cpu_dst_64); >> break; > > Use gen_ne_fop_DFF. Good catch, I clearly missed that this can use the new helper, I will respin this one. Thanks, Nick
Re: [PATCH 8/8] target/sparc: Fix VIS subtraction instructions.
On 2023-09-28, Richard Henderson wrote: > On 9/24/23 01:03, Nick Bowler wrote: >> All of the VIS subtraction instructions are documented to subtract the >> second input operand from the first. This is also consistent with how >> the instructions actually work on a real UltraSparc II. >> >> But the emulator is implementing the subtraction in the wrong order, >> subtracting the first input from the second, so the results are wrong >> in all nontrivial cases. >> >> Signed-off-by: Nick Bowler >> --- >> target/sparc/vis_helper.c | 18 +- >> 1 file changed, 9 insertions(+), 9 deletions(-) > > While this patch works, better to use > > void tcg_gen_vec_add16_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b); > void tcg_gen_vec_add16_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b); > void tcg_gen_vec_add32_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b); > > void tcg_gen_vec_sub16_i32(TCGv_i32 d, TCGv_i32 a, TCGv_i32 b); > void tcg_gen_vec_sub16_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b); > void tcg_gen_vec_sub32_i64(TCGv_i64 d, TCGv_i64 a, TCGv_i64 b); > > from "tcg/tcg-op-gvec.h" and remove the sparc helpers. OK, I will try to respin this one using these. Thanks, Nick
[PATCH 7/8] target/sparc: Fix VIS fexpand input register.
This instruction is documented to get its input from the second
single-precision input operand; the first operand is ignored.
This is exactly what a real UltraSparc II does. Meanwhile, the
the emulator uses only the irrelevant first operand, treating
it as a double-precision register, and ignores the second.
This will not normally contain the correct data so the emulated
instruction usually just produces garbage.
Signed-off-by: Nick Bowler
---
target/sparc/helper.h | 2 +-
target/sparc/translate.c | 5 -
target/sparc/vis_helper.c | 5 ++---
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index b71688079f..81d44e7618 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -133,7 +133,7 @@ DEF_HELPER_FLAGS_2(fmul8sux16, TCG_CALL_NO_RWG_SE, i64,
i64, i64)
DEF_HELPER_FLAGS_2(fmul8ulx16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmuld8sux16, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fmuld8ulx16, TCG_CALL_NO_RWG_SE, i64, i32, i32)
-DEF_HELPER_FLAGS_2(fexpand, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_1(fexpand, TCG_CALL_NO_RWG_SE, i64, i32)
DEF_HELPER_FLAGS_3(pdist, TCG_CALL_NO_RWG_SE, i64, i64, i64, i64)
DEF_HELPER_FLAGS_2(fpack16, TCG_CALL_NO_RWG_SE, i32, i64, i64)
DEF_HELPER_FLAGS_3(fpack32, TCG_CALL_NO_RWG_SE, i64, i64, i64, i64)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 241ac429ca..4e92c27768 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -4837,7 +4837,10 @@ static void disas_sparc_insn(DisasContext * dc, unsigned
int insn)
break;
case 0x04d: /* VIS I fexpand */
CHECK_FPU_FEATURE(dc, VIS1);
-gen_ne_fop_DDD(dc, rd, rs1, rs2, gen_helper_fexpand);
+cpu_src2_32 = gen_load_fpr_F(dc, rs2);
+cpu_dst_64 = gen_dest_fpr_D(dc, rd);
+gen_helper_fexpand(cpu_dst_64, cpu_src2_32);
+gen_store_fpr_D(dc, rd, cpu_dst_64);
break;
case 0x050: /* VIS I fpadd16 */
CHECK_FPU_FEATURE(dc, VIS1);
diff --git a/target/sparc/vis_helper.c b/target/sparc/vis_helper.c
index 029aad3923..3903beaf5d 100644
--- a/target/sparc/vis_helper.c
+++ b/target/sparc/vis_helper.c
@@ -260,13 +260,12 @@ uint64_t helper_fmuld8ulx16(uint32_t src1, uint32_t src2)
return d.ll;
}
-uint64_t helper_fexpand(uint64_t src1, uint64_t src2)
+uint64_t helper_fexpand(uint32_t src2)
{
VIS32 s;
VIS64 d;
-s.l = (uint32_t)src1;
-d.ll = src2;
+s.l = src2;
d.VIS_W64(0) = s.VIS_B32(0) << 4;
d.VIS_W64(1) = s.VIS_B32(1) << 4;
d.VIS_W64(2) = s.VIS_B32(2) << 4;
--
2.41.0
[PATCH 1/8] target/sparc: Fix VIS fmul8x16 input register.
On a real UltraSparc II CPU, the fmul8x16 instruction reads its first
input from any of the single-precision floating point registers.
But the emulator is reading the input as if the first operand encodes
a double-precision register, which in most cases will not contain the
right data and therefore the output of the emulated instruction is
just garbage.
Signed-off-by: Nick Bowler
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1901
---
target/sparc/helper.h | 2 +-
target/sparc/translate.c | 6 +-
target/sparc/vis_helper.c | 9 +
3 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index b8f1e78c75..ace731a22c 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -126,7 +126,7 @@ DEF_HELPER_FLAGS_2(fdtox, TCG_CALL_NO_RWG, s64, env, f64)
DEF_HELPER_FLAGS_1(fqtox, TCG_CALL_NO_RWG, s64, env)
DEF_HELPER_FLAGS_2(fpmerge, TCG_CALL_NO_RWG_SE, i64, i64, i64)
-DEF_HELPER_FLAGS_2(fmul8x16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(fmul8x16, TCG_CALL_NO_RWG_SE, i64, i32, i64)
DEF_HELPER_FLAGS_2(fmul8x16al, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmul8x16au, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmul8sux16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 3bf0ab8135..bb65b8daf8 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -4750,7 +4750,11 @@ static void disas_sparc_insn(DisasContext * dc, unsigned
int insn)
break;
case 0x031: /* VIS I fmul8x16 */
CHECK_FPU_FEATURE(dc, VIS1);
-gen_ne_fop_DDD(dc, rd, rs1, rs2, gen_helper_fmul8x16);
+cpu_src1_32 = gen_load_fpr_F(dc, rs1);
+cpu_src2_64 = gen_load_fpr_D(dc, rs2);
+cpu_dst_64 = gen_dest_fpr_D(dc, rd);
+gen_helper_fmul8x16(cpu_dst_64, cpu_src1_32, cpu_src2_64);
+gen_store_fpr_D(dc, rd, cpu_dst_64);
break;
case 0x033: /* VIS I fmul8x16au */
CHECK_FPU_FEATURE(dc, VIS1);
diff --git a/target/sparc/vis_helper.c b/target/sparc/vis_helper.c
index 3afdc6975c..d158b39b85 100644
--- a/target/sparc/vis_helper.c
+++ b/target/sparc/vis_helper.c
@@ -94,16 +94,17 @@ uint64_t helper_fpmerge(uint64_t src1, uint64_t src2)
return d.ll;
}
-uint64_t helper_fmul8x16(uint64_t src1, uint64_t src2)
+uint64_t helper_fmul8x16(uint32_t src1, uint64_t src2)
{
-VIS64 s, d;
+VIS32 s;
+VIS64 d;
uint32_t tmp;
-s.ll = src1;
+s.l = src1;
d.ll = src2;
#define PMUL(r) \
-tmp = (int32_t)d.VIS_SW64(r) * (int32_t)s.VIS_B64(r); \
+tmp = (int32_t)d.VIS_SW64(r) * (int32_t)s.VIS_B32(r); \
if ((tmp & 0xff) > 0x7f) { \
tmp += 0x100; \
} \
--
2.41.0
[PATCH 3/8] target/sparc: Fix VIS fmul8x16al instruction.
On a real UltraSparc II, the fmul8x16al instruction takes two single-
precision input operands and returns a double-precision result. For
the second operand, bits 15:0 are used, and bits 31:16 are ignored.
However, the emulation is taking two double-precision input operands,
and furthermore it is using bits 31:16 of the second operand (ignoring
bits 15:0). These are unlikely to contain the correct values.
Even still, the emulator overwrites the second input before all outputs
are calculated, so even if by chance the data loaded in happens to be
correct, the results are just garbage except in trivial cases.
Signed-off-by: Nick Bowler
---
target/sparc/helper.h | 2 +-
target/sparc/translate.c | 2 +-
target/sparc/vis_helper.c | 11 ++-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index 76e06b8ea5..25d6178ca5 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -127,7 +127,7 @@ DEF_HELPER_FLAGS_1(fqtox, TCG_CALL_NO_RWG, s64, env)
DEF_HELPER_FLAGS_2(fpmerge, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmul8x16, TCG_CALL_NO_RWG_SE, i64, i32, i64)
-DEF_HELPER_FLAGS_2(fmul8x16al, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(fmul8x16al, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fmul8x16au, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fmul8sux16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmul8ulx16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index ca81b35a25..dddee9f974 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -4779,7 +4779,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned
int insn)
break;
case 0x035: /* VIS I fmul8x16al */
CHECK_FPU_FEATURE(dc, VIS1);
-gen_ne_fop_DDD(dc, rd, rs1, rs2, gen_helper_fmul8x16al);
+gen_ne_fop_DFF(dc, rd, rs1, rs2, gen_helper_fmul8x16al);
break;
case 0x036: /* VIS I fmul8sux16 */
CHECK_FPU_FEATURE(dc, VIS1);
diff --git a/target/sparc/vis_helper.c b/target/sparc/vis_helper.c
index 2fc783a054..386cfd0706 100644
--- a/target/sparc/vis_helper.c
+++ b/target/sparc/vis_helper.c
@@ -122,16 +122,17 @@ uint64_t helper_fmul8x16(uint32_t src1, uint64_t src2)
return d.ll;
}
-uint64_t helper_fmul8x16al(uint64_t src1, uint64_t src2)
+uint64_t helper_fmul8x16al(uint32_t src1, uint32_t src2)
{
-VIS64 s, d;
+VIS32 s1, s2;
+VIS64 d;
uint32_t tmp;
-s.ll = src1;
-d.ll = src2;
+s1.l = src1;
+s2.l = src2;
#define PMUL(r) \
-tmp = (int32_t)d.VIS_SW64(1) * (int32_t)s.VIS_B64(r); \
+tmp = (int32_t)s2.VIS_SW32(0) * (int32_t)s1.VIS_B64(r); \
if ((tmp & 0xff) > 0x7f) { \
tmp += 0x100; \
} \
--
2.41.0
[PATCH 8/8] target/sparc: Fix VIS subtraction instructions.
All of the VIS subtraction instructions are documented to subtract the second input operand from the first. This is also consistent with how the instructions actually work on a real UltraSparc II. But the emulator is implementing the subtraction in the wrong order, subtracting the first input from the second, so the results are wrong in all nontrivial cases. Signed-off-by: Nick Bowler --- target/sparc/vis_helper.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/target/sparc/vis_helper.c b/target/sparc/vis_helper.c index 3903beaf5d..fa97e09ffa 100644 --- a/target/sparc/vis_helper.c +++ b/target/sparc/vis_helper.c @@ -282,10 +282,10 @@ uint64_t helper_fexpand(uint32_t src2) s.ll = src1;\ d.ll = src2;\ \ -d.VIS_W64(0) = F(d.VIS_W64(0), s.VIS_W64(0)); \ -d.VIS_W64(1) = F(d.VIS_W64(1), s.VIS_W64(1)); \ -d.VIS_W64(2) = F(d.VIS_W64(2), s.VIS_W64(2)); \ -d.VIS_W64(3) = F(d.VIS_W64(3), s.VIS_W64(3)); \ +d.VIS_W64(0) = F(s.VIS_W64(0), d.VIS_W64(0)); \ +d.VIS_W64(1) = F(s.VIS_W64(1), d.VIS_W64(1)); \ +d.VIS_W64(2) = F(s.VIS_W64(2), d.VIS_W64(2)); \ +d.VIS_W64(3) = F(s.VIS_W64(3), d.VIS_W64(3)); \ \ return d.ll;\ } \ @@ -297,8 +297,8 @@ uint64_t helper_fexpand(uint32_t src2) s.l = src1; \ d.l = src2; \ \ -d.VIS_W32(0) = F(d.VIS_W32(0), s.VIS_W32(0)); \ -d.VIS_W32(1) = F(d.VIS_W32(1), s.VIS_W32(1)); \ +d.VIS_W32(0) = F(s.VIS_W32(0), d.VIS_W32(0)); \ +d.VIS_W32(1) = F(s.VIS_W32(1), d.VIS_W32(1)); \ \ return d.l; \ } \ @@ -310,8 +310,8 @@ uint64_t helper_fexpand(uint32_t src2) s.ll = src1;\ d.ll = src2;\ \ -d.VIS_L64(0) = F(d.VIS_L64(0), s.VIS_L64(0)); \ -d.VIS_L64(1) = F(d.VIS_L64(1), s.VIS_L64(1)); \ +d.VIS_L64(0) = F(s.VIS_L64(0), d.VIS_L64(0)); \ +d.VIS_L64(1) = F(s.VIS_L64(1), d.VIS_L64(1)); \ \ return d.ll;\ } \ @@ -323,7 +323,7 @@ uint64_t helper_fexpand(uint32_t src2) s.l = src1; \ d.l = src2; \ \ -d.l = F(d.l, s.l); \ +d.l = F(s.l, d.l); \ \ return d.l; \ } -- 2.41.0
[PATCH 6/8] target/sparc: Fix VIS fpmerge input registers.
On a real UltraSparc II CPU, the fpmerge instruction reads two
single-precision input registers, but the emulator is reading
from double-precision input registers instead.
These are unlikely to contain the correct data so in most instances
the results of the emulation are just garbage in most instances.
Signed-off-by: Nick Bowler
---
target/sparc/helper.h | 2 +-
target/sparc/translate.c | 6 +-
target/sparc/vis_helper.c | 26 +-
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index 7a588f3068..b71688079f 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -125,7 +125,7 @@ DEF_HELPER_FLAGS_2(fstox, TCG_CALL_NO_RWG, s64, env, f32)
DEF_HELPER_FLAGS_2(fdtox, TCG_CALL_NO_RWG, s64, env, f64)
DEF_HELPER_FLAGS_1(fqtox, TCG_CALL_NO_RWG, s64, env)
-DEF_HELPER_FLAGS_2(fpmerge, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(fpmerge, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fmul8x16, TCG_CALL_NO_RWG_SE, i64, i32, i64)
DEF_HELPER_FLAGS_2(fmul8x16al, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fmul8x16au, TCG_CALL_NO_RWG_SE, i64, i32, i32)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index cfccd95c3a..241ac429ca 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -4825,7 +4825,11 @@ static void disas_sparc_insn(DisasContext * dc, unsigned
int insn)
break;
case 0x04b: /* VIS I fpmerge */
CHECK_FPU_FEATURE(dc, VIS1);
-gen_ne_fop_DDD(dc, rd, rs1, rs2, gen_helper_fpmerge);
+cpu_src1_32 = gen_load_fpr_F(dc, rs1);
+cpu_src2_32 = gen_load_fpr_F(dc, rs2);
+cpu_dst_64 = gen_dest_fpr_D(dc, rd);
+gen_helper_fpmerge(cpu_dst_64, cpu_src1_32, cpu_src2_32);
+gen_store_fpr_D(dc, rd, cpu_dst_64);
break;
case 0x04c: /* VIS II bshuffle */
CHECK_FPU_FEATURE(dc, VIS2);
diff --git a/target/sparc/vis_helper.c b/target/sparc/vis_helper.c
index 306383ba60..029aad3923 100644
--- a/target/sparc/vis_helper.c
+++ b/target/sparc/vis_helper.c
@@ -77,22 +77,22 @@ typedef union {
float32 f;
} VIS32;
-uint64_t helper_fpmerge(uint64_t src1, uint64_t src2)
+uint64_t helper_fpmerge(uint32_t src1, uint32_t src2)
{
-VIS64 s, d;
+VIS32 s1, s2;
+VIS64 d;
-s.ll = src1;
-d.ll = src2;
+s1.l = src1;
+s2.l = src2;
-/* Reverse calculation order to handle overlap */
-d.VIS_B64(7) = s.VIS_B64(3);
-d.VIS_B64(6) = d.VIS_B64(3);
-d.VIS_B64(5) = s.VIS_B64(2);
-d.VIS_B64(4) = d.VIS_B64(2);
-d.VIS_B64(3) = s.VIS_B64(1);
-d.VIS_B64(2) = d.VIS_B64(1);
-d.VIS_B64(1) = s.VIS_B64(0);
-/* d.VIS_B64(0) = d.VIS_B64(0); */
+d.VIS_B64(0) = s2.VIS_B32(0);
+d.VIS_B64(1) = s1.VIS_B32(0);
+d.VIS_B64(2) = s2.VIS_B32(1);
+d.VIS_B64(3) = s1.VIS_B32(1);
+d.VIS_B64(4) = s2.VIS_B32(2);
+d.VIS_B64(5) = s1.VIS_B32(2);
+d.VIS_B64(6) = s2.VIS_B32(3);
+d.VIS_B64(7) = s1.VIS_B32(3);
return d.ll;
}
--
2.41.0
[PATCH 4/8] target/sparc: Fix VIS fmuld8sux16 instruction.
On a real UltraSparc II, the fmuld8sux16 instruction takes two single-
precision input operands and returns a double-precision result.
However, the emulation is taking two double-precision input operands,
which are unlikely to contain the correct values. Even if they did,
the emulator is rounding the output, which the real processor does
not do. And the real processor shifts both outputs left by 8, which
the emulator does not do.
So the results are wrong except in trivial cases.
Signed-off-by: Nick Bowler
---
target/sparc/helper.h | 2 +-
target/sparc/translate.c | 2 +-
target/sparc/vis_helper.c | 19 ---
3 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index 25d6178ca5..adc1ea6653 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -131,7 +131,7 @@ DEF_HELPER_FLAGS_2(fmul8x16al, TCG_CALL_NO_RWG_SE, i64,
i32, i32)
DEF_HELPER_FLAGS_2(fmul8x16au, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fmul8sux16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmul8ulx16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
-DEF_HELPER_FLAGS_2(fmuld8sux16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(fmuld8sux16, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fmuld8ulx16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fexpand, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_3(pdist, TCG_CALL_NO_RWG_SE, i64, i64, i64, i64)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index dddee9f974..1017d3bca7 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -4791,7 +4791,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned
int insn)
break;
case 0x038: /* VIS I fmuld8sux16 */
CHECK_FPU_FEATURE(dc, VIS1);
-gen_ne_fop_DDD(dc, rd, rs1, rs2, gen_helper_fmuld8sux16);
+gen_ne_fop_DFF(dc, rd, rs1, rs2, gen_helper_fmuld8sux16);
break;
case 0x039: /* VIS I fmuld8ulx16 */
CHECK_FPU_FEATURE(dc, VIS1);
diff --git a/target/sparc/vis_helper.c b/target/sparc/vis_helper.c
index 386cfd0706..de5ddad39a 100644
--- a/target/sparc/vis_helper.c
+++ b/target/sparc/vis_helper.c
@@ -220,24 +220,21 @@ uint64_t helper_fmul8ulx16(uint64_t src1, uint64_t src2)
return d.ll;
}
-uint64_t helper_fmuld8sux16(uint64_t src1, uint64_t src2)
+uint64_t helper_fmuld8sux16(uint32_t src1, uint32_t src2)
{
-VIS64 s, d;
+VIS32 s1, s2;
+VIS64 d;
uint32_t tmp;
-s.ll = src1;
-d.ll = src2;
+s1.l = src1;
+s2.l = src2;
#define PMUL(r) \
-tmp = (int32_t)d.VIS_SW64(r) * ((int32_t)s.VIS_SW64(r) >> 8); \
-if ((tmp & 0xff) > 0x7f) { \
-tmp += 0x100; \
-} \
-d.VIS_L64(r) = tmp;
+tmp = (int32_t)s2.VIS_SW32(r) * ((int32_t)s1.VIS_SW32(r) >> 8); \
+d.VIS_L64(r) = tmp << 8;
-/* Reverse calculation order to handle overlap */
-PMUL(1);
PMUL(0);
+PMUL(1);
#undef PMUL
return d.ll;
--
2.41.0
[PATCH 2/8] target/sparc: Fix VIS fmul8x16au instruction.
On a real UltraSparc II, the fmul8x16au instruction takes two single-
precision input operands and returns a double-precision result. For
the second operand, bits 31:16 are used, and bits 15:0 are ignored.
However, the emulation is taking two double-precision input operands,
and furthermore it is using bits 15:0 of the second operand (ignoring
bits 31:16). These are unlikely to contain the correct values.
Even still, the emulator overwrites the second input before all outputs
are calculated, so even if by chance the data loaded in happens to be
correct, the results are just garbage except in trivial cases.
Signed-off-by: Nick Bowler
---
target/sparc/helper.h | 2 +-
target/sparc/translate.c | 19 ++-
target/sparc/vis_helper.c | 14 +-
3 files changed, 28 insertions(+), 7 deletions(-)
diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index ace731a22c..76e06b8ea5 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -128,7 +128,7 @@ DEF_HELPER_FLAGS_1(fqtox, TCG_CALL_NO_RWG, s64, env)
DEF_HELPER_FLAGS_2(fpmerge, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmul8x16, TCG_CALL_NO_RWG_SE, i64, i32, i64)
DEF_HELPER_FLAGS_2(fmul8x16al, TCG_CALL_NO_RWG_SE, i64, i64, i64)
-DEF_HELPER_FLAGS_2(fmul8x16au, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(fmul8x16au, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fmul8sux16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmul8ulx16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmuld8sux16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index bb65b8daf8..ca81b35a25 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -1786,6 +1786,23 @@ static void gen_fop_DFF(DisasContext *dc, int rd, int
rs1, int rs2,
gen_store_fpr_D(dc, rd, dst);
}
+#ifdef TARGET_SPARC64
+static void gen_ne_fop_DFF(DisasContext *dc, int rd, int rs1, int rs2,
+ void (*gen)(TCGv_i64, TCGv_i32, TCGv_i32))
+{
+TCGv_i64 dst;
+TCGv_i32 src1, src2;
+
+src1 = gen_load_fpr_F(dc, rs1);
+src2 = gen_load_fpr_F(dc, rs2);
+dst = gen_dest_fpr_D(dc, rd);
+
+gen(dst, src1, src2);
+
+gen_store_fpr_D(dc, rd, dst);
+}
+#endif
+
static void gen_fop_QDD(DisasContext *dc, int rd, int rs1, int rs2,
void (*gen)(TCGv_ptr, TCGv_i64, TCGv_i64))
{
@@ -4758,7 +4775,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned
int insn)
break;
case 0x033: /* VIS I fmul8x16au */
CHECK_FPU_FEATURE(dc, VIS1);
-gen_ne_fop_DDD(dc, rd, rs1, rs2, gen_helper_fmul8x16au);
+gen_ne_fop_DFF(dc, rd, rs1, rs2, gen_helper_fmul8x16au);
break;
case 0x035: /* VIS I fmul8x16al */
CHECK_FPU_FEATURE(dc, VIS1);
diff --git a/target/sparc/vis_helper.c b/target/sparc/vis_helper.c
index d158b39b85..2fc783a054 100644
--- a/target/sparc/vis_helper.c
+++ b/target/sparc/vis_helper.c
@@ -49,6 +49,7 @@ target_ulong helper_array8(target_ulong pixel_addr,
target_ulong cubesize)
#define VIS_L64(n) l[1 - (n)]
#define VIS_B32(n) b[3 - (n)]
#define VIS_W32(n) w[1 - (n)]
+#define VIS_SW32(n) sw[1 - (n)]
#else
#define VIS_B64(n) b[n]
#define VIS_W64(n) w[n]
@@ -56,6 +57,7 @@ target_ulong helper_array8(target_ulong pixel_addr,
target_ulong cubesize)
#define VIS_L64(n) l[n]
#define VIS_B32(n) b[n]
#define VIS_W32(n) w[n]
+#define VIS_SW32(n) sw[n]
#endif
typedef union {
@@ -70,6 +72,7 @@ typedef union {
typedef union {
uint8_t b[4];
uint16_t w[2];
+int16_t sw[2];
uint32_t l;
float32 f;
} VIS32;
@@ -143,16 +146,17 @@ uint64_t helper_fmul8x16al(uint64_t src1, uint64_t src2)
return d.ll;
}
-uint64_t helper_fmul8x16au(uint64_t src1, uint64_t src2)
+uint64_t helper_fmul8x16au(uint32_t src1, uint32_t src2)
{
-VIS64 s, d;
+VIS32 s1, s2;
+VIS64 d;
uint32_t tmp;
-s.ll = src1;
-d.ll = src2;
+s1.l = src1;
+s2.l = src2;
#define PMUL(r) \
-tmp = (int32_t)d.VIS_SW64(0) * (int32_t)s.VIS_B64(r); \
+tmp = (int32_t)s2.VIS_SW32(1) * (int32_t)s1.VIS_B64(r); \
if ((tmp & 0xff) > 0x7f) { \
tmp += 0x100; \
} \
--
2.41.0
[PATCH 5/8] target/sparc: Fix VIS fmuld8ulx16 instruction.
On a real UltraSparc II, the fmuld8ulx16 instruction takes two single-
precision input operands and returns a double-precision result.
However, the emulation is taking two double-precision input operands,
which are unlikely to contain the correct values, so the results are
garbage in most cases. Even if the inputs happen to be correct, the
emulator is rounding the output, which the real processor does not do.
Signed-off-by: Nick Bowler
---
target/sparc/helper.h | 2 +-
target/sparc/translate.c | 2 +-
target/sparc/vis_helper.c | 17 +++--
3 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/target/sparc/helper.h b/target/sparc/helper.h
index adc1ea6653..7a588f3068 100644
--- a/target/sparc/helper.h
+++ b/target/sparc/helper.h
@@ -132,7 +132,7 @@ DEF_HELPER_FLAGS_2(fmul8x16au, TCG_CALL_NO_RWG_SE, i64,
i32, i32)
DEF_HELPER_FLAGS_2(fmul8sux16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmul8ulx16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_2(fmuld8sux16, TCG_CALL_NO_RWG_SE, i64, i32, i32)
-DEF_HELPER_FLAGS_2(fmuld8ulx16, TCG_CALL_NO_RWG_SE, i64, i64, i64)
+DEF_HELPER_FLAGS_2(fmuld8ulx16, TCG_CALL_NO_RWG_SE, i64, i32, i32)
DEF_HELPER_FLAGS_2(fexpand, TCG_CALL_NO_RWG_SE, i64, i64, i64)
DEF_HELPER_FLAGS_3(pdist, TCG_CALL_NO_RWG_SE, i64, i64, i64, i64)
DEF_HELPER_FLAGS_2(fpack16, TCG_CALL_NO_RWG_SE, i32, i64, i64)
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 1017d3bca7..cfccd95c3a 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -4795,7 +4795,7 @@ static void disas_sparc_insn(DisasContext * dc, unsigned
int insn)
break;
case 0x039: /* VIS I fmuld8ulx16 */
CHECK_FPU_FEATURE(dc, VIS1);
-gen_ne_fop_DDD(dc, rd, rs1, rs2, gen_helper_fmuld8ulx16);
+gen_ne_fop_DFF(dc, rd, rs1, rs2, gen_helper_fmuld8ulx16);
break;
case 0x03a: /* VIS I fpack32 */
CHECK_FPU_FEATURE(dc, VIS1);
diff --git a/target/sparc/vis_helper.c b/target/sparc/vis_helper.c
index de5ddad39a..306383ba60 100644
--- a/target/sparc/vis_helper.c
+++ b/target/sparc/vis_helper.c
@@ -240,24 +240,21 @@ uint64_t helper_fmuld8sux16(uint32_t src1, uint32_t src2)
return d.ll;
}
-uint64_t helper_fmuld8ulx16(uint64_t src1, uint64_t src2)
+uint64_t helper_fmuld8ulx16(uint32_t src1, uint32_t src2)
{
-VIS64 s, d;
+VIS32 s1, s2;
+VIS64 d;
uint32_t tmp;
-s.ll = src1;
-d.ll = src2;
+s1.l = src1;
+s2.l = src2;
#define PMUL(r) \
-tmp = (int32_t)d.VIS_SW64(r) * ((uint32_t)s.VIS_B64(r * 2));\
-if ((tmp & 0xff) > 0x7f) { \
-tmp += 0x100; \
-} \
+tmp = (int32_t)s2.VIS_SW32(r) * ((uint32_t)s1.VIS_B32(r * 2)); \
d.VIS_L64(r) = tmp;
-/* Reverse calculation order to handle overlap */
-PMUL(1);
PMUL(0);
+PMUL(1);
#undef PMUL
return d.ll;
--
2.41.0
[PATCH 0/8] SPARC VIS fixes
I noticed that the fmul8x16 instruction did not appear to be emulated correctly[1]. It would seem that emulation was not using a single- precision input register like the real hardware does, but rather a double-precision register, causing it to operate on the wrong data. Every other VIS instruction which contains one or more single-precision inputs and a double-precision output has the exact same problem. A few computational problems are found and fixed by this series too. All patches can be applied independently, except patch 2 adds some helpers which are subsequently needed by patches 3, 4 and 5. Emulation results are tested by manually comparing the output of a small Linux test program on an UltraSparc II against the output of running the same binary under qemu-sparc32plus on a ppc64le host system. [1] https://gitlab.com/qemu-project/qemu/-/issues/1901 Nick Bowler (8): target/sparc: Fix VIS fmul8x16 input register. target/sparc: Fix VIS fmul8x16au instruction. target/sparc: Fix VIS fmul8x16al instruction. target/sparc: Fix VIS fmuld8sux16 instruction. target/sparc: Fix VIS fmuld8ulx16 instruction. target/sparc: Fix VIS fpmerge input registers. target/sparc: Fix VIS fexpand input register. target/sparc: Fix VIS subtraction instructions. target/sparc/helper.h | 14 ++--- target/sparc/translate.c | 42 +++--- target/sparc/vis_helper.c | 119 +++--- 3 files changed, 101 insertions(+), 74 deletions(-) -- 2.41.0
Re: NetBSD and libfdt (was: Re: MSYS2 and libfdt)
hi, On 24/01/2023 11:27, Thomas Huth wrote: On 24/01/2023 10.20, Thomas Huth wrote: [...] On Thu, Jan 19, 2023 at 12:31 PM Thomas Huth wrote: Hi all, in some spare minutes, I started playing with a patch to try to remove the dtc submodule from the QEMU git repository - according to https://repology.org/project/dtc/versions our supported build platforms should now all provide the minimum required version. [...] Ok, I'll give my patch another try to see whether all the other systems have a usable version of libfdt available, too. ... and I apparently missed NetBSD in my first research: Looks like NetBSD is still using dtc v1.4.7 which is too old for QEMU. (though https://www.netbsd.org/docs/software/3rdparty/ talks about v1.5.1, I only get dtc 1.4.7 in our NetBSD VM). The not yet released netbsd-10 and -current have 1.5.1. Perhaps you can use netbsd-10 for your VM? Thanks, Nick
[PATCH v2] hw/net: e1000e: Clear ICR on read when using non MSI-X interrupts
In section 7.4.3 of the 82574 datasheet it states that
"In systems that do not support MSI-X, reading the ICR
register clears it's bits..."
Some OSes rely on this.
Signed-off-by: Nick Hudson
---
hw/net/e1000e_core.c | 5 +
hw/net/trace-events | 1 +
2 files changed, 6 insertions(+)
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 8ae6fb7e14..2c51089a82 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -2607,6 +2607,11 @@ e1000e_mac_icr_read(E1000ECore *core, int index)
core->mac[ICR] = 0;
}
+if (!msix_enabled(core->owner)) {
+trace_e1000e_irq_icr_clear_nonmsix_icr_read();
+core->mac[ICR] = 0;
+}
+
if ((core->mac[ICR] & E1000_ICR_ASSERTED) &&
(core->mac[CTRL_EXT] & E1000_CTRL_EXT_IAME)) {
trace_e1000e_irq_icr_clear_iame();
diff --git a/hw/net/trace-events b/hw/net/trace-events
index 643338f610..4c0ec3fda1 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -221,6 +221,7 @@ e1000e_irq_write_ics(uint32_t val) "Adding ICR bits 0x%x"
e1000e_irq_icr_process_iame(void) "Clearing IMS bits due to IAME"
e1000e_irq_read_ics(uint32_t ics) "Current ICS: 0x%x"
e1000e_irq_read_ims(uint32_t ims) "Current IMS: 0x%x"
+e1000e_irq_icr_clear_nonmsix_icr_read(void) "Clearing ICR on read due to non
MSI-X int"
e1000e_irq_icr_read_entry(uint32_t icr) "Starting ICR read. Current ICR: 0x%x"
e1000e_irq_icr_read_exit(uint32_t icr) "Ending ICR read. Current ICR: 0x%x"
e1000e_irq_icr_clear_zero_ims(void) "Clearing ICR on read due to zero IMS"
--
2.25.1
[PATCH] hw/net: e1000e: Clear ICR on read when using non MSI-X interrupts
In section 7.4.3 of the 82574 datasheet it states that
"In systems that do not support MSI-X, reading the ICR
register clears it's bits..."
Some OSes rely on this.
Signed-off-by: Nick Hudson
---
hw/net/e1000e_core.c | 5 +
hw/net/trace-events | 1 +
2 files changed, 6 insertions(+)
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 8ae6fb7e14..2c51089a82 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -2607,6 +2607,11 @@ e1000e_mac_icr_read(E1000ECore *core, int index)
core->mac[ICR] = 0;
}
+if (!msix_enabled(core->owner)) {
+trace_e1000e_irq_icr_clear_nonmsix_icr_read();
+core->mac[ICR] = 0;
+}
+
if ((core->mac[ICR] & E1000_ICR_ASSERTED) &&
(core->mac[CTRL_EXT] & E1000_CTRL_EXT_IAME)) {
trace_e1000e_irq_icr_clear_iame();
diff --git a/hw/net/trace-events b/hw/net/trace-events
index 643338f610..084086ec44 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -221,6 +221,7 @@ e1000e_irq_write_ics(uint32_t val) "Adding ICR bits 0x%x"
e1000e_irq_icr_process_iame(void) "Clearing IMS bits due to IAME"
e1000e_irq_read_ics(uint32_t ics) "Current ICS: 0x%x"
e1000e_irq_read_ims(uint32_t ims) "Current IMS: 0x%x"
+e1000e_irq_icr_clear_nonmisx_icr_read(void) "Clearing ICR on read due to non
MSI-X int"
e1000e_irq_icr_read_entry(uint32_t icr) "Starting ICR read. Current ICR: 0x%x"
e1000e_irq_icr_read_exit(uint32_t icr) "Ending ICR read. Current ICR: 0x%x"
e1000e_irq_icr_clear_zero_ims(void) "Clearing ICR on read due to zero IMS"
--
2.25.1
request for wiki account
hello there! i've been using qemu since about 2005, but somehow never needed to make an account on the wiki. i now do. can i get an account created for 'dankamongmen' or, if you prefer real names, 'nickblack'? thanks! -- nick black -=- https://www.nick-black.com to make an apple pie from scratch, you need first invent a universe. signature.asc Description: PGP signature
Re: [PATCH v5 17/31] target/arm: Enforce alignment for LDM/STM
On Tue, Sep 7, 2021 at 6:44 AM Richard Henderson
wrote:
>
> On 8/31/21 2:51 AM, Nathan Chancellor wrote:
> > I just bisected a boot hang with an LLVM-built multi_v7_defconfig +
> > CONFIG_THUMB2_KERNEL=y kernel down to this commit. I do not see the same
> > hang when the kernel is compiled with GCC 11.2.0 and binutils 2.37 nor
> > do I see a hang with multi_v7_defconfig by itself. Is there something
> > that LLVM is doing wrong when compiling/assembling/linking the kernel or
> > is there something wrong/too aggressive with this commit? I can
> > reproduce this with current QEMU HEAD (ad22d05833).
> >
> > My QEMU invocation is:
> >
> > $ qemu-system-arm \
> > -append "console=ttyAMA0 earlycon" \
> > -display none \
> > -initrd rootfs.cpio \
> > -kernel zImage \
> > -M virt \
> > -m 512m \
> > -nodefaults \
> > -no-reboot \
> > -serial mon:stdio
> >
> > and the rootfs.cpio and zImage files can be found here:
> >
> > https://github.com/nathanchance/bug-files/tree/15c1fd6e44622a3c27823d2c5c3083dfc7246146/qemu-2e1f39e29bf9a6b28eaee9fc0949aab50dbad94a
>
> Hmm. I see
>
> IN:
> 0xc13038e2: e890 008c ldm.wr0, {r2, r3, r7}
>
> R00=c13077ca R01=c11a8058 R02=c11a8058 R03=c031737f
> R04=48379000 R05=0024 R06=c031748d R07=c03174bb
> R08=412fc0f1 R09=c0ce9308 R10=50c5387d R11=
> R12=0009 R13=c1501f88 R14=c0301739 R15=c13038e2
> PSR=21f3 --C- T svc32
> Taking exception 4 [Data Abort]
> ...from EL1 to EL1
> ...with ESR 0x25/0x963f
> ...with DFSR 0x1 DFAR 0xc13077ca
>
> So, yes, it's a ldm from an address % 4 = 2, so it is correct that we should
> trap. You
> should see the same trap on real hw.
Makes sense. I guess if we can find which label that's in, we can look
closer into the code generated by the compiler.
scripts/extract-vmlinux doesn't seem to be able to extract a vmlinux
from either zImage artifact though; not sure yet we'll be able to
disassemble those.
Oh, I guess GDB can show us. Looks like 0xc13038e2 is...hard to tell,
there's no debug info so we just have jumps to addresses in hex, not
symbols. I don't know my way around GDB well enough to get a sense
for where we are in the source code.
https://gist.github.com/nickdesaulniers/764ac9afab04775846ffa6c90c5a266a
If I rebuild QEMU from source, I don't get any disassembly that looks
similar, probably as a result of different compiler versions, and
maybe adding debug info.
--
Thanks,
~Nick Desaulniers
Re: [PATCH] target/arm: Correct the encoding of MDCCSR_EL0
> On 29 Jun 2021, at 12:50, Peter Maydell wrote:
>
> On Tue, 29 Jun 2021 at 11:41, Nick Hudson wrote:
>>
>>
>>
>>> On 29 Jun 2021, at 10:49, Peter Maydell wrote:
>>>
>>> On Tue, 29 Jun 2021 at 09:27, wrote:
>>>>
>>>> Signed-off-by: Nick Hudson
>>>> ---
>>>> target/arm/helper.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>>> index a66c1f0b9e..7267af7924 100644
>>>> --- a/target/arm/helper.c
>>>> +++ b/target/arm/helper.c
>>>> @@ -6330,7 +6330,7 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>>>> * We don't implement the configurable EL0 access.
>>>> */
>>>>{ .name = "MDCCSR_EL0", .state = ARM_CP_STATE_BOTH,
>>>> - .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
>>>> + .cp = 14, .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
>>>> .type = ARM_CP_ALIAS,
>>>> .access = PL1_R, .accessfn = access_tda,
>>>> .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
>>>
>>> This fixes the encoding for AArch64, but breaks it for AArch32,
>>> where it is cp=14 opc1=0 crn=0 crm=1 opc2=0. Because this is one of
>>> those system registers where the AArch64 and AArch32 encodings
>>> don't match up, to fix the AArch64 encoding we need to replace
>>> this ARM_CP_STATE_BOTH reginfo with separate reginfo for
>>> ARM_CP_STATE_AA32 and ARM_CP_STATE_AA64, something like this:
>>>
>>> { .name = "MDCCSR_EL0", .state = ARM_CP_STATE_AA64,
>>> .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
>>> .type = ARM_CP_ALIAS,
>>> .access = PL1_R, .accessfn = access_tda,
>>> .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
>>> { .name = "DBGDSCRint", .state = ARM_CP_STATE_AA32,
>>> .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
>>> .type = ARM_CP_ALIAS,
>>> .access = PL1_R, .accessfn = access_tda,
>>> .fieldoffset = offsetoflow32(CPUARMState, cp15.mdscr_el1), },
>>>
>>
>> Ah, yes.
>>
>> As MDCCSR_EL0[30:29] only maps to DBGDSCRint[30:29] maybe it should be all
>> RAZ?
>
> Well, you can't make it all RAZ, because those 2 bits do still
> need to be mapped, but I guess in theory yes we should define
> read and write accessor functions for AArch64 MDCCSR_EL0 that
> mask out everything except [30:29].
(Apologies if you get this/similar twice - my email is doing strange things)
Hi Peter,
I think the following is acceptable in that qemu doesn’t touch MDSCR_EL1 as far
as I can tell.
Perhaps I’m reading the code and the ARM ARM wrong?
/* MDCCSR_EL0[30:29] map to DBGDSCRint[30:29]. Simply RAZ.
* We don't implement the configurable EL0 access.
*/
{ .name = "MDCCSR_EL0", .state = ARM_CP_STATE_AA64,
.cp = 14, .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
.type = ARM_CP_CONST, .resetvalue = 0 },
/* DBGDSCRint[15,12,5:2] map to MDSCR_EL1[15,12,5:2] */
{ .name = "DBGDSCRint", .state = ARM_CP_STATE_AA32,
.cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
.type = ARM_CP_ALIAS,
.access = PL1_R, .accessfn = access_tda,
.fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
Please let me know if you want me to post this (or a different change) as a new
diff.
Thanks,
Nick
Re: [PATCH] target/arm: Correct the encoding of MDCCSR_EL0
> On 29 Jun 2021, at 12:50, Peter Maydell wrote:
>
> On Tue, 29 Jun 2021 at 11:41, Nick Hudson wrote:
>>
>>
>>
>>> On 29 Jun 2021, at 10:49, Peter Maydell wrote:
>>>
>>> On Tue, 29 Jun 2021 at 09:27, wrote:
>>>>
>>>> Signed-off-by: Nick Hudson
>>>> ---
>>>> target/arm/helper.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>>>> index a66c1f0b9e..7267af7924 100644
>>>> --- a/target/arm/helper.c
>>>> +++ b/target/arm/helper.c
>>>> @@ -6330,7 +6330,7 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>>>> * We don't implement the configurable EL0 access.
>>>> */
>>>>{ .name = "MDCCSR_EL0", .state = ARM_CP_STATE_BOTH,
>>>> - .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
>>>> + .cp = 14, .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
>>>> .type = ARM_CP_ALIAS,
>>>> .access = PL1_R, .accessfn = access_tda,
>>>> .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
>>>
>>> This fixes the encoding for AArch64, but breaks it for AArch32,
>>> where it is cp=14 opc1=0 crn=0 crm=1 opc2=0. Because this is one of
>>> those system registers where the AArch64 and AArch32 encodings
>>> don't match up, to fix the AArch64 encoding we need to replace
>>> this ARM_CP_STATE_BOTH reginfo with separate reginfo for
>>> ARM_CP_STATE_AA32 and ARM_CP_STATE_AA64, something like this:
>>>
>>> { .name = "MDCCSR_EL0", .state = ARM_CP_STATE_AA64,
>>> .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
>>> .type = ARM_CP_ALIAS,
>>> .access = PL1_R, .accessfn = access_tda,
>>> .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
>>> { .name = "DBGDSCRint", .state = ARM_CP_STATE_AA32,
>>> .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
>>> .type = ARM_CP_ALIAS,
>>> .access = PL1_R, .accessfn = access_tda,
>>> .fieldoffset = offsetoflow32(CPUARMState, cp15.mdscr_el1), },
>>>
>>
>> Ah, yes.
>>
>> As MDCCSR_EL0[30:29] only maps to DBGDSCRint[30:29] maybe it should be all
>> RAZ?
>
> Well, you can't make it all RAZ, because those 2 bits do still
> need to be mapped, but I guess in theory yes we should define
> read and write accessor functions for AArch64 MDCCSR_EL0 that
> mask out everything except [30:29].
Hi Peter,
Maybe I’m misreading the ARM ARM and the qemu use of mdscr_el1, but I think
this is good enough / more correct. I’m somewhat confused by AA64 MDSCR_EL1
vs DBGSCRint vs DBGSCRext, however.
/* MDCCSR_EL0[30:29] map to DBGDSCRint[30:29]. Simply RAZ.
* We don't implement the configurable EL0 access.
*/
{ .name = "MDCCSR_EL0", .state = ARM_CP_STATE_AA64,
.cp = 14, .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
.type = ARM_CP_CONST, .resetvalue = 0 },
/* DBGDSCRint[15,12,5:2] map to MDSCR_EL1[15,12,5:2] */
{ .name = "DBGDSCRint", .state = ARM_CP_STATE_AA32,
.cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
.type = ARM_CP_ALIAS,
.access = PL1_R, .accessfn = access_tda,
.fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
Please let me know if you want me to submit a new patch.
Thanks,
Nick
[PATCH] target/arm: Correct the encoding of MDCCSR_EL0
Signed-off-by: Nick Hudson
---
target/arm/helper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index a66c1f0b9e..7267af7924 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6330,7 +6330,7 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
* We don't implement the configurable EL0 access.
*/
{ .name = "MDCCSR_EL0", .state = ARM_CP_STATE_BOTH,
- .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
+ .cp = 14, .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
.type = ARM_CP_ALIAS,
.access = PL1_R, .accessfn = access_tda,
.fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
--
2.31.1
Re: [PATCH] target/arm: Correct the encoding of MDCCSR_EL0
> On 29 Jun 2021, at 10:49, Peter Maydell wrote:
>
> On Tue, 29 Jun 2021 at 09:27, wrote:
>>
>> Signed-off-by: Nick Hudson
>> ---
>> target/arm/helper.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/arm/helper.c b/target/arm/helper.c
>> index a66c1f0b9e..7267af7924 100644
>> --- a/target/arm/helper.c
>> +++ b/target/arm/helper.c
>> @@ -6330,7 +6330,7 @@ static const ARMCPRegInfo debug_cp_reginfo[] = {
>> * We don't implement the configurable EL0 access.
>> */
>> { .name = "MDCCSR_EL0", .state = ARM_CP_STATE_BOTH,
>> - .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
>> + .cp = 14, .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
>> .type = ARM_CP_ALIAS,
>> .access = PL1_R, .accessfn = access_tda,
>> .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
>
> This fixes the encoding for AArch64, but breaks it for AArch32,
> where it is cp=14 opc1=0 crn=0 crm=1 opc2=0. Because this is one of
> those system registers where the AArch64 and AArch32 encodings
> don't match up, to fix the AArch64 encoding we need to replace
> this ARM_CP_STATE_BOTH reginfo with separate reginfo for
> ARM_CP_STATE_AA32 and ARM_CP_STATE_AA64, something like this:
>
>{ .name = "MDCCSR_EL0", .state = ARM_CP_STATE_AA64,
> .opc0 = 2, .opc1 = 3, .crn = 0, .crm = 1, .opc2 = 0,
> .type = ARM_CP_ALIAS,
> .access = PL1_R, .accessfn = access_tda,
> .fieldoffset = offsetof(CPUARMState, cp15.mdscr_el1), },
>{ .name = "DBGDSCRint", .state = ARM_CP_STATE_AA32,
> .cp = 14, .opc0 = 2, .opc1 = 0, .crn = 0, .crm = 1, .opc2 = 0,
> .type = ARM_CP_ALIAS,
> .access = PL1_R, .accessfn = access_tda,
> .fieldoffset = offsetoflow32(CPUARMState, cp15.mdscr_el1), },
>
Ah, yes.
As MDCCSR_EL0[30:29] only maps to DBGDSCRint[30:29] maybe it should be all RAZ?
Thanks,
Nick
[Bug 1917565] Re: Windows 10 fails with "Boot device inaccessible"
I haven't tried a new install. Also, Michael asked to check
git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
The code in question is changed there and it works fine for that existing image.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1917565
Title:
Windows 10 fails with "Boot device inaccessible"
Status in QEMU:
New
Bug description:
The issue is happening on all versions I tried after the following
commit. I can also remove this individual change from master and it
starts to work.
OVMF_CODE.fd is what comes with Ubuntu 20.04 through package manager.
git diff af1b80ae56c9495999e8ccf7b70ef894378de642~
af1b80ae56c9495999e8ccf7b70ef894378de642
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bc2a..7a5a8b3521 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
dev = aml_device("PCI0");
aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
aml_append(sb_scope, dev);
aml_append(dsdt, sb_scope);
@@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
aml_append(dev, build_q35_osc_method());
aml_append(sb_scope, dev);
aml_append(dsdt, sb_scope);
The virtual machine start command:
x86_64-softmmu/qemu-system-x86_64 -name guest=win10-dev,debug-threads=on
-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/win10-dev_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
-machine
pc-q35-4.2,accel=kvm,usb=off,vmport=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format
-cpu
Skylake-Client-IBRS,ss=on,vmx=on,pdcm=on,hypervisor=on,tsc-adjust=on,clflushopt=on,umip=on,md-clear=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,pdpe1gb=on,ibpb=on,amd-ssbd=on,skip-l1dfl-vmentry=on,pschange-mc-no=on,hv-time,hv-relaxed,hv-vapic,hv-spinlocks=0x1fff
-m 6144 -overcommit mem-lock=off -smp 4,sockets=4,cores=1,threads=1 -uuid
5646e540-5022-4ace-8d6a-d7c4b61a6d3d -no-user-config -nodefaults -rtc
base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet
-global ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 -boot strict=on
-device
pcie-root-port,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x2
-device pcie-root-port,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1
-device pcie-root-port,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2
-device pcie-root-port,port=0x13,chassis=4,id=pci.4,bus=pcie.0,addr=0x2.0x3
-device pcie-root-port,port=0x14,chassis=5,id=pci.5,bus=pcie.0,addr=0x2.0x4
-device qemu-xhci,p2=15,p3=15,id=usb,bus=pci.2,addr=0x0 -device
virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 -blockdev
'{"driver":"host_device","filename":"/dev/disk/by-id/scsi-1SanDisk_Extreme_SSD_20072F404043","aio":"native","node-name":"libvirt-2-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-2-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"raw","file":"libvirt-2-storage"}'
-device
ide-hd,bus=ide.0,drive=libvirt-2-format,id=sata0-0-0,bootindex=1,write-cache=on
-device ide-cd,bus=ide.1,id=sata0-0-1 -netdev user,id=hostnet0 -device
e1000e,netdev=hostnet0,id=net0,mac=52:54:00:10:5b:55,bus=pci.1,addr=0x0
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0
-chardev spicevmc,id=charchannel0,name=vdagent -device
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
-device usb-tablet,id=input0,bus=usb.0,port=1 -spice
port=5900,addr=127.0.0.1,disable-ticketing=on,image-compression=off,seamless-migration=on
-device
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pcie.0,addr=0x1
-device ich9-intel-hda,id=so
[Bug 1917565] Re: Windows 10 fails with "Boot device inaccessible"
** Description changed:
The issue is happening on all versions I tried after the following
commit. I can also remove this individual change from master and it
starts to work.
+
+ OVMF_CODE.fd is what comes with Ubuntu 20.04 through package manager.
+
git diff af1b80ae56c9495999e8ccf7b70ef894378de642~
af1b80ae56c9495999e8ccf7b70ef894378de642
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bc2a..7a5a8b3521 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
dev = aml_device("PCI0");
aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
aml_append(sb_scope, dev);
aml_append(dsdt, sb_scope);
@@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
aml_append(dev, build_q35_osc_method());
aml_append(sb_scope, dev);
aml_append(dsdt, sb_scope);
The virtual machine start command:
x86_64-softmmu/qemu-system-x86_64 -name guest=win10-dev,debug-threads=on
-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/win10-dev_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
-machine
pc-q35-4.2,accel=kvm,usb=off,vmport=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format
-cpu
Skylake-Client-IBRS,ss=on,vmx=on,pdcm=on,hypervisor=on,tsc-adjust=on,clflushopt=on,umip=on,md-clear=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,pdpe1gb=on,ibpb=on,amd-ssbd=on,skip-l1dfl-vmentry=on,pschange-mc-no=on,hv-time,hv-relaxed,hv-vapic,hv-spinlocks=0x1fff
-m 6144 -overcommit mem-lock=off -smp 4,sockets=4,cores=1,threads=1 -uuid
5646e540-5022-4ace-8d6a-d7c4b61a6d3d -no-user-config -nodefaults -rtc
base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet
-global ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 -boot strict=on
-device
pcie-root-port,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x2
-device pcie-root-port,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1
-device pcie-root-port,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2
-device pcie-root-port,port=0x13,chassis=4,id=pci.4,bus=pcie.0,addr=0x2.0x3
-device pcie-root-port,port=0x14,chassis=5,id=pci.5,bus=pcie.0,addr=0x2.0x4
-device qemu-xhci,p2=15,p3=15,id=usb,bus=pci.2,addr=0x0 -device
virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 -blockdev
'{"driver":"host_device","filename":"/dev/disk/by-id/scsi-1SanDisk_Extreme_SSD_20072F404043","aio":"native","node-name":"libvirt-2-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-2-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"raw","file":"libvirt-2-storage"}'
-device
ide-hd,bus=ide.0,drive=libvirt-2-format,id=sata0-0-0,bootindex=1,write-cache=on
-device ide-cd,bus=ide.1,id=sata0-0-1 -netdev user,id=hostnet0 -device
e1000e,netdev=hostnet0,id=net0,mac=52:54:00:10:5b:55,bus=pci.1,addr=0x0
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0
-chardev spicevmc,id=charchannel0,name=vdagent -device
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
-device usb-tablet,id=input0,bus=usb.0,port=1 -spice
port=5900,addr=127.0.0.1,disable-ticketing=on,image-compression=off,seamless-migration=on
-device
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pcie.0,addr=0x1
-device ich9-intel-hda,id=sound0,bus=pcie.0,addr=0x1b -device
hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev
spicevmc,id=charredir0,name=usbredir -device
usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=2 -chardev
spicevmc,id=charredir1,name=usbredir -device
usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=3 -device
virtio-balloon-pci,id=balloon0,bus=pci.4,addr=0x0 -msg timestamp=on -D
./log.txt -monitor stdio -d
--
You received
[Bug 1917565] Re: Windows 10 fails with "Boot device inaccessible"
** Description changed:
The issue is happening on all versions I tried after the following
- commit.
+ commit. I can also remove this individual from master and it starts to
+ work.
git diff af1b80ae56c9495999e8ccf7b70ef894378de642~
af1b80ae56c9495999e8ccf7b70ef894378de642
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bc2a..7a5a8b3521 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
- dev = aml_device("PCI0");
- aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
- aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
+ dev = aml_device("PCI0");
+ aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
+ aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
- aml_append(sb_scope, dev);
- aml_append(dsdt, sb_scope);
-
+ aml_append(sb_scope, dev);
+ aml_append(dsdt, sb_scope);
+
@@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
- aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
- aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
- aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
+ aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
+ aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
+ aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
- aml_append(dev, build_q35_osc_method());
- aml_append(sb_scope, dev);
- aml_append(dsdt, sb_scope);
+ aml_append(dev, build_q35_osc_method());
+ aml_append(sb_scope, dev);
+ aml_append(dsdt, sb_scope);
The virtual machine start command:
x86_64-softmmu/qemu-system-x86_64 -name guest=win10-dev,debug-threads=on
-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/win10-dev_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
-machine
pc-q35-4.2,accel=kvm,usb=off,vmport=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format
-cpu
Skylake-Client-IBRS,ss=on,vmx=on,pdcm=on,hypervisor=on,tsc-adjust=on,clflushopt=on,umip=on,md-clear=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,pdpe1gb=on,ibpb=on,amd-ssbd=on,skip-l1dfl-vmentry=on,pschange-mc-no=on,hv-time,hv-relaxed,hv-vapic,hv-spinlocks=0x1fff
-m 6144 -overcommit mem-lock=off -smp 4,sockets=4,cores=1,threads=1 -uuid
5646e540-5022-4ace-8d6a-d7c4b61a6d3d -no-user-config -nodefaults -rtc
base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet
-global ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 -boot strict=on
-device
pcie-root-port,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x2
-device pcie-root-port,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1
-device pcie-root-port,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2
-device pcie-root-port,port=0x13,chassis=4,id=pci.4,bus=pcie.0,addr=0x2.0x3
-device pcie-root-port,port=0x14,chassis=5,id=pci.5,bus=pcie.0,addr=0x2.0x4
-device qemu-xhci,p2=15,p3=15,id=usb,bus=pci.2,addr=0x0 -device
virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 -blockdev
'{"driver":"host_device","filename":"/dev/disk/by-id/scsi-1SanDisk_Extreme_SSD_20072F404043","aio":"native","node-name":"libvirt-2-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-2-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"raw","file":"libvirt-2-storage"}'
-device
ide-hd,bus=ide.0,drive=libvirt-2-format,id=sata0-0-0,bootindex=1,write-cache=on
-device ide-cd,bus=ide.1,id=sata0-0-1 -netdev user,id=hostnet0 -device
e1000e,netdev=hostnet0,id=net0,mac=52:54:00:10:5b:55,bus=pci.1,addr=0x0
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0
-chardev spicevmc,id=charchannel0,name=vdagent -device
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
-device usb-tablet,id=input0,bus=usb.0,port=1 -spice
port=5900,addr=127.0.0.1,disable-ticketing=on,image-compression=off,seamless-migration=on
-device
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram6
[Bug 1917565] [NEW] Windows 10 fails with "Boot device inaccessible"
Public bug reported:
The issue is happening on all versions I tried after the following
commit.
git diff af1b80ae56c9495999e8ccf7b70ef894378de642~
af1b80ae56c9495999e8ccf7b70ef894378de642
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bc2a..7a5a8b3521 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1497,7 +1497,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
dev = aml_device("PCI0");
aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A03")));
aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
aml_append(sb_scope, dev);
aml_append(dsdt, sb_scope);
@@ -1512,7 +1512,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
aml_append(dev, aml_name_decl("_CID", aml_eisaid("PNP0A03")));
aml_append(dev, aml_name_decl("_ADR", aml_int(0)));
-aml_append(dev, aml_name_decl("_UID", aml_int(1)));
+aml_append(dev, aml_name_decl("_UID", aml_int(0)));
aml_append(dev, build_q35_osc_method());
aml_append(sb_scope, dev);
aml_append(dsdt, sb_scope);
The virtual machine start command:
x86_64-softmmu/qemu-system-x86_64 -name guest=win10-dev,debug-threads=on
-blockdev
'{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","node-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-pflash0-format","read-only":true,"driver":"raw","file":"libvirt-pflash0-storage"}'
-blockdev
'{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/win10-dev_VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-pflash1-format","read-only":false,"driver":"raw","file":"libvirt-pflash1-storage"}'
-machine
pc-q35-4.2,accel=kvm,usb=off,vmport=off,dump-guest-core=off,pflash0=libvirt-pflash0-format,pflash1=libvirt-pflash1-format
-cpu
Skylake-Client-IBRS,ss=on,vmx=on,pdcm=on,hypervisor=on,tsc-adjust=on,clflushopt=on,umip=on,md-clear=on,stibp=on,arch-capabilities=on,ssbd=on,xsaves=on,pdpe1gb=on,ibpb=on,amd-ssbd=on,skip-l1dfl-vmentry=on,pschange-mc-no=on,hv-time,hv-relaxed,hv-vapic,hv-spinlocks=0x1fff
-m 6144 -overcommit mem-lock=off -smp 4,sockets=4,cores=1,threads=1 -uuid
5646e540-5022-4ace-8d6a-d7c4b61a6d3d -no-user-config -nodefaults -rtc
base=localtime,driftfix=slew -global kvm-pit.lost_tick_policy=delay -no-hpet
-global ICH9-LPC.disable_s3=1 -global ICH9-LPC.disable_s4=1 -boot strict=on
-device
pcie-root-port,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,addr=0x2
-device pcie-root-port,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1
-device pcie-root-port,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2
-device pcie-root-port,port=0x13,chassis=4,id=pci.4,bus=pcie.0,addr=0x2.0x3
-device pcie-root-port,port=0x14,chassis=5,id=pci.5,bus=pcie.0,addr=0x2.0x4
-device qemu-xhci,p2=15,p3=15,id=usb,bus=pci.2,addr=0x0 -device
virtio-serial-pci,id=virtio-serial0,bus=pci.3,addr=0x0 -blockdev
'{"driver":"host_device","filename":"/dev/disk/by-id/scsi-1SanDisk_Extreme_SSD_20072F404043","aio":"native","node-name":"libvirt-2-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}'
-blockdev
'{"node-name":"libvirt-2-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"raw","file":"libvirt-2-storage"}'
-device
ide-hd,bus=ide.0,drive=libvirt-2-format,id=sata0-0-0,bootindex=1,write-cache=on
-device ide-cd,bus=ide.1,id=sata0-0-1 -netdev user,id=hostnet0 -device
e1000e,netdev=hostnet0,id=net0,mac=52:54:00:10:5b:55,bus=pci.1,addr=0x0
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0
-chardev spicevmc,id=charchannel0,name=vdagent -device
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=com.redhat.spice.0
-device usb-tablet,id=input0,bus=usb.0,port=1 -spice
port=5900,addr=127.0.0.1,disable-ticketing=on,image-compression=off,seamless-migration=on
-device
qxl-vga,id=video0,ram_size=67108864,vram_size=67108864,vram64_size_mb=0,vgamem_mb=16,max_outputs=1,bus=pcie.0,addr=0x1
-device ich9-intel-hda,id=sound0,bus=pcie.0,addr=0x1b -device
hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 -chardev
spicevmc,id=charredir0,name=usbredir -device
usb-redir,chardev=charredir0,id=redir0,bus=usb.0,port=2 -chardev
spicevmc,id=charredir1,name=usbredir -device
usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=3 -device
virtio-balloon-pci,id=balloon0,bus=pci.4,addr=0x0 -msg timestamp=on -D
./log.txt -monitor stdio -d
** Affects: qemu
Importance: Undecided
Status: New
** Tags: 10 boot device inaccessible windows
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https:/
Re: [PATCH] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute
Did this happen to get picked up already? EOM
On Thu, Feb 11, 2021 at 11:43 AM Nathan Chancellor wrote:
>
> fw_cfg_showrev() is called by an indirect call in kobj_attr_show(),
> which violates clang's CFI checking because fw_cfg_showrev()'s second
> parameter is 'struct attribute', whereas the ->show() member of 'struct
> kobj_structure' expects the second parameter to be of type 'struct
> kobj_attribute'.
>
> $ cat /sys/firmware/qemu_fw_cfg/rev
> 3
>
> $ dmesg | grep "CFI failure"
> [ 26.016832] CFI failure (target: fw_cfg_showrev+0x0/0x8):
>
> Fix this by converting fw_cfg_rev_attr to 'struct kobj_attribute' where
> this would have been caught automatically by the incompatible pointer
> types compiler warning. Update fw_cfg_showrev() accordingly.
>
> Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg
> device")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1299
> Signed-off-by: Nathan Chancellor
> ---
> drivers/firmware/qemu_fw_cfg.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 0078260fbabe..172c751a4f6c 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -299,15 +299,13 @@ static int fw_cfg_do_platform_probe(struct
> platform_device *pdev)
> return 0;
> }
>
> -static ssize_t fw_cfg_showrev(struct kobject *k, struct attribute *a, char
> *buf)
> +static ssize_t fw_cfg_showrev(struct kobject *k, struct kobj_attribute *a,
> + char *buf)
> {
> return sprintf(buf, "%u\n", fw_cfg_rev);
> }
>
> -static const struct {
> - struct attribute attr;
> - ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf);
> -} fw_cfg_rev_attr = {
> +static const struct kobj_attribute fw_cfg_rev_attr = {
> .attr = { .name = "rev", .mode = S_IRUSR },
> .show = fw_cfg_showrev,
> };
>
> base-commit: 92bf22614b21a2706f4993b278017e437f7785b3
> --
> 2.30.1
>
> --
> You received this message because you are subscribed to the Google Groups
> "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to clang-built-linux+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/clang-built-linux/20210211194258.4137998-1-nathan%40kernel.org.
--
Thanks,
~Nick Desaulniers
Re: [PATCH] usb-host: use correct altsetting in usb_host_ep_update
On Thu, Feb 18, 2021 at 12:52:51PM +0100, Gerd Hoffmann wrote: > On Thu, Feb 11, 2021 at 11:05:41AM -0500, Nick Rosbrook wrote: > > Hi, > > > > Just wanted to ping this. Patchwork link is here: > > https://patchwork.kernel.org/project/qemu-devel/patch/20210201213021.500277-1-rosbro...@ainfosec.com/. > > Pull request sent now. > Not much usb activity these days ... Thanks! NR
Re: [PATCH] usb-host: use correct altsetting in usb_host_ep_update
Hi,
Just wanted to ping this. Patchwork link is here:
https://patchwork.kernel.org/project/qemu-devel/patch/20210201213021.500277-1-rosbro...@ainfosec.com/.
Thanks,
NR
On Mon, Feb 1, 2021 at 4:30 PM Nick Rosbrook wrote:
>
> In order to keep track of the alternate setting that should be used for
> a given interface, the USBDevice struct keeps an array of alternate
> setting values, which is indexed by the interface number. In
> usb_host_set_interface, when this array is updated, usb_host_ep_update
> is called as a result. However, when usb_host_ep_update accesses the
> active libusb_config_descriptor, it indexes udev->altsetting with the
> loop variable, rather than the interface number.
>
> With the simple trace backend enable, this behavior can be seen:
>
> [...]
>
> usb_xhci_xfer_start 0.440 pid=1215 xfer=0x5596a4b85930 slotid=0x1 epid=0x1
> streamid=0x0
> usb_packet_state_change 1.703 pid=1215 bus=0x1 port=b'1' ep=0x0
> p=0x5596a4b85938 o=b'undef' n=b'setup'
> usb_host_req_control 2.269 pid=1215 bus=0x1 addr=0x5 p=0x5596a4b85938
> req=0x10b value=0x1 index=0xd
> usb_host_set_interface 0.449 pid=1215 bus=0x1 addr=0x5 interface=0xd alt=0x1
> usb_host_parse_config 2542.648 pid=1215 bus=0x1 addr=0x5 value=0x2
> active=0x1
> usb_host_parse_interface 1.804 pid=1215 bus=0x1 addr=0x5 num=0xc alt=0x0
> active=0x1
> usb_host_parse_endpoint 2.012 pid=1215 bus=0x1 addr=0x5 ep=0x2 dir=b'in'
> type=b'int' active=0x1
> usb_host_parse_interface 1.598 pid=1215 bus=0x1 addr=0x5 num=0xd alt=0x0
> active=0x1
> usb_host_req_emulated 3.593 pid=1215 bus=0x1 addr=0x5 p=0x5596a4b85938
> status=0x0
> usb_packet_state_change 2.550 pid=1215 bus=0x1 port=b'1' ep=0x0
> p=0x5596a4b85938 o=b'setup' n=b'complete'
> usb_xhci_xfer_success 4.298 pid=1215 xfer=0x5596a4b85930 bytes=0x0
>
> [...]
>
> In particular, it is seen that although usb_host_set_interface sets the
> alternate setting of interface 0xd to 0x1, usb_host_ep_update uses 0x0
> as the alternate setting due to using the incorrect index to
> udev->altsetting.
>
> Fix this problem by getting the interface number from the active
> libusb_config_descriptor, and then using that as the index to
> udev->altsetting.
>
> Signed-off-by: Nick Rosbrook
> ---
> hw/usb/host-libusb.c | 18 +++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
> index fcf48c0193..6ab75e2feb 100644
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -810,7 +810,7 @@ static void usb_host_ep_update(USBHostDevice *s)
> struct libusb_ss_endpoint_companion_descriptor *endp_ss_comp;
> #endif
> uint8_t devep, type;
> -int pid, ep;
> +int pid, ep, alt;
> int rc, i, e;
>
> usb_ep_reset(udev);
> @@ -822,8 +822,20 @@ static void usb_host_ep_update(USBHostDevice *s)
> conf->bConfigurationValue, true);
>
> for (i = 0; i < conf->bNumInterfaces; i++) {
> -assert(udev->altsetting[i] < conf->interface[i].num_altsetting);
> -intf = &conf->interface[i].altsetting[udev->altsetting[i]];
> +/*
> + * The udev->altsetting array indexes alternate settings
> + * by the interface number. Get the 0th alternate setting
> + * first so that we can grab the interface number, and
> + * then correct the alternate setting value if necessary.
> + */
> +intf = &conf->interface[i].altsetting[0];
> +alt = udev->altsetting[intf->bInterfaceNumber];
> +
> +if (alt != 0) {
> +assert(alt < conf->interface[i].num_altsetting);
> +intf = &conf->interface[i].altsetting[alt];
> +}
> +
> trace_usb_host_parse_interface(s->bus_num, s->addr,
> intf->bInterfaceNumber,
> intf->bAlternateSetting, true);
> --
> 2.17.1
>
[PATCH] usb-host: use correct altsetting in usb_host_ep_update
In order to keep track of the alternate setting that should be used for
a given interface, the USBDevice struct keeps an array of alternate
setting values, which is indexed by the interface number. In
usb_host_set_interface, when this array is updated, usb_host_ep_update
is called as a result. However, when usb_host_ep_update accesses the
active libusb_config_descriptor, it indexes udev->altsetting with the
loop variable, rather than the interface number.
With the simple trace backend enable, this behavior can be seen:
[...]
usb_xhci_xfer_start 0.440 pid=1215 xfer=0x5596a4b85930 slotid=0x1 epid=0x1
streamid=0x0
usb_packet_state_change 1.703 pid=1215 bus=0x1 port=b'1' ep=0x0
p=0x5596a4b85938 o=b'undef' n=b'setup'
usb_host_req_control 2.269 pid=1215 bus=0x1 addr=0x5 p=0x5596a4b85938
req=0x10b value=0x1 index=0xd
usb_host_set_interface 0.449 pid=1215 bus=0x1 addr=0x5 interface=0xd alt=0x1
usb_host_parse_config 2542.648 pid=1215 bus=0x1 addr=0x5 value=0x2 active=0x1
usb_host_parse_interface 1.804 pid=1215 bus=0x1 addr=0x5 num=0xc alt=0x0
active=0x1
usb_host_parse_endpoint 2.012 pid=1215 bus=0x1 addr=0x5 ep=0x2 dir=b'in'
type=b'int' active=0x1
usb_host_parse_interface 1.598 pid=1215 bus=0x1 addr=0x5 num=0xd alt=0x0
active=0x1
usb_host_req_emulated 3.593 pid=1215 bus=0x1 addr=0x5 p=0x5596a4b85938
status=0x0
usb_packet_state_change 2.550 pid=1215 bus=0x1 port=b'1' ep=0x0
p=0x5596a4b85938 o=b'setup' n=b'complete'
usb_xhci_xfer_success 4.298 pid=1215 xfer=0x5596a4b85930 bytes=0x0
[...]
In particular, it is seen that although usb_host_set_interface sets the
alternate setting of interface 0xd to 0x1, usb_host_ep_update uses 0x0
as the alternate setting due to using the incorrect index to
udev->altsetting.
Fix this problem by getting the interface number from the active
libusb_config_descriptor, and then using that as the index to
udev->altsetting.
Signed-off-by: Nick Rosbrook
---
hw/usb/host-libusb.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index fcf48c0193..6ab75e2feb 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -810,7 +810,7 @@ static void usb_host_ep_update(USBHostDevice *s)
struct libusb_ss_endpoint_companion_descriptor *endp_ss_comp;
#endif
uint8_t devep, type;
-int pid, ep;
+int pid, ep, alt;
int rc, i, e;
usb_ep_reset(udev);
@@ -822,8 +822,20 @@ static void usb_host_ep_update(USBHostDevice *s)
conf->bConfigurationValue, true);
for (i = 0; i < conf->bNumInterfaces; i++) {
-assert(udev->altsetting[i] < conf->interface[i].num_altsetting);
-intf = &conf->interface[i].altsetting[udev->altsetting[i]];
+/*
+ * The udev->altsetting array indexes alternate settings
+ * by the interface number. Get the 0th alternate setting
+ * first so that we can grab the interface number, and
+ * then correct the alternate setting value if necessary.
+ */
+intf = &conf->interface[i].altsetting[0];
+alt = udev->altsetting[intf->bInterfaceNumber];
+
+if (alt != 0) {
+assert(alt < conf->interface[i].num_altsetting);
+intf = &conf->interface[i].altsetting[alt];
+}
+
trace_usb_host_parse_interface(s->bus_num, s->addr,
intf->bInterfaceNumber,
intf->bAlternateSetting, true);
--
2.17.1
Re: [PATCH v1] s390x/tcg: Fix RISBHG
On Fri, Jan 8, 2021 at 1:45 AM David Hildenbrand wrote:
>
> On 08.01.21 03:20, Nick Desaulniers wrote:
> > On Thu, Jan 7, 2021 at 3:27 PM David Hildenbrand
> > wrote:
> >>
> >>
> >>> Am 08.01.2021 um 00:21 schrieb Nick Desaulniers :
> >>>
> >>> On Thu, Jan 7, 2021 at 3:13 PM David Hildenbrand
> >>> wrote:
> >>>>
> >>>> RISBHG is broken and currently hinders clang builds of upstream kernels
> >>>> from booting: the kernel crashes early, while decompressing the image.
> >>>>
> >>>> [...]
> >>>> Kernel fault: interruption code 0005 ilc:2
> >>>> Kernel random base:
> >>>> PSW : 20018000 00017a1e
> >>>> R:0 T:0 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:2 PM:0 RI:0 EA:3
> >>>> GPRS: 0001 000c 0003fff4
> >>>> fff0
> >>>> fff4 000c
> >>>> fff0
> >>>> fffc fff8
> >>>> 008e25a8
> >>>> 0009 0002 0008
> >>>> bce0
> >>>>
> >>>> One example of a buggy instruction is:
> >>>>
> >>>>17dde: ec 1e 00 9f 20 5d risbhg %r1,%r14,0,159,32
> >>>>
> >>>> With %r14 = 0x9 and %r1 = 0x7 should result in %r1 = 0x90007,
> >>>> however,
> >>>> results in %r1 = 0.
> >>>>
> >>>> Let's interpret values of i3/i4 as documented in the PoP and make
> >>>> computation of "mask" only based on i3 and i4 and use "pmask" only at the
> >>>> very end to make sure wrapping is only applied to the high/low
> >>>> doubleword.
> >>>>
> >>>> With this patch, I can successfully boot a v5.10 kernel built with
> >>>> clang, and gcc builds keep on working.
> >>>>
> >>>> Fixes: 2d6a869833d9 ("target-s390: Implement RISBG")
> >>>> Reported-by: Nick Desaulniers
> >>>> Cc: Guenter Roeck
> >>>> Cc: Christian Borntraeger
> >>>> Signed-off-by: David Hildenbrand
> >>>> ---
> >>>>
> >>>> This BUG was a nightmare to debug and the code a nightmare to understand.
> >>>>
> >>>> To make clang/gcc builds boot, the following fix is required as well on
> >>>> top of current master: "[PATCH] target/s390x: Fix ALGSI"
> >>>> https://lkml.kernel.org/r/20210107202135.52379-1-da...@redhat.com
> >>>
> >>> In that case, a huge thank you!!! for this work! ++beers_owed.
> >>>
> >>
> >> :) a kernel build for z13 should work with the (default) „-cpu qemu“ cpu
> >> type.
> >
> > Hmm...so I don't think clang can build a Linux kernel image with
> > CONFIG_MARCH_Z13=y just yet; just defconfig. Otherwise looks like
> > clang barfs on some of the inline asm constraints.
> >
>
> Ah, right. I overwrote my manual config by a temporary defconfig :)
>
>
> So, I'm on x86-64 F33.
>
> clang version 11.0.0 (Fedora 11.0.0-2.fc33)
> LLVM version 11.0.0
>
> I cannot directly use "LLVM=1" for cross-compilation, as I keep getting
> "error: unknown emulation: elf64_s390" from ld.lld and "error: invalid
> output format: 'elf64-s390'" from llvm-objcopy. I assume that's fixed in
> llvm12?
Right, I suspect that even if ld.lld understood that emulation mode
target, it would still fail due to lack of big endian support. We've
been building with simply `CC=clang` for s390 linux kernels.
Via: https://www.kernel.org/doc/html/latest/kbuild/llvm.html#llvm-utilities
we usually start with `make CC=clang` then work our way up to `make
LLVM=1`. So you shouldn't need the below patching, just use
`CC=clang`.
>
> 1. I patch around it (strange, I remember CC= .. used to work, but it no
> longer does)
>
> ---
>
> index e30cf02da8b8..89c57062ed5d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -427,13 +427,13 @@ KBUILD_HOSTLDLIBS := $(HOST_LFS_LIBS) $(HOSTLDLIBS)
> CPP= $(CC) -E
> ifneq ($(LLVM),)
> CC = clang
> -LD = ld.lld
> -AR = llvm-ar
> -NM = llvm-nm
> -OBJCOPY= llvm-objcopy
> -OBJDUMP= llvm-objdump
> -READELF= llvm-readelf
> -STRIP = llvm-strip
> +LD = $(CROSS_COMPILE)ld
> +AR = $(CROSS_COMPILE)ar
> +NM = $(CROSS_COMPILE)nm
> +OBJCOPY= $(CROSS_COMPILE)objcopy
> +OBJDUMP= $(CROSS_COMPILE)objdump
> +READELF= $(CROSS_COMPILE)readelf
> +STRIP = $(CROSS_COMPILE)strip
> else
> CC = $(CROSS_COMPILE)gcc
> LD = $(CROSS_COMPILE)ld
>
> ---
Pulling from your github branch, everything looks good; buildroot
support looks good. I'll wire this up to our CI so that we can help
report regressions!
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2 0/4] s390x/tcg: fix booting Linux kernels compiled with clang-11 and clang-12
On Fri, Jan 8, 2021 at 5:21 AM David Hildenbrand wrote: > > This series fixes booting current upstream Linux kernel compiled by > clang-11 and clang-12 under TCG. > > Decided to pull in already separatly sent patches. The last patch is > not required to fix the boot issues, but related to patch #3. > > Latest version of the patches available at: > g...@github.com:davidhildenbrand/qemu.git clang Hey looks like we're off to the races! $ qemu/build/qemu-system-s390x -M s390-ccw-virtio -display none -initrd /android1/boot-utils/images/s390/rootfs.cpio -kernel /android0/linux-next/arch/s390/boot/bzImage -m 512m -nodefaults -serial mon:stdio ... [0.365077] Linux version 5.11.0-rc2-01914-g16586f130181-dirty (ndesaulni...@ndesaulniers1.mtv.corp.google.com) (Nick Desaulniers clang version 12.0.0 (g...@github.com:llvm/llvm-project.git e75fec2b238f0e26cfb7645f2208baebe3440d41), GNU ld (GNU Binutils for Debian) 2.35.1) #76 SMP Thu Jan 7 17:51:34 PST 2021 ... / # cat /proc/version Linux version 5.11.0-rc2-01914-g16586f130181-dirty (ndesaulni...@ndesaulniers1.mtv.corp.google.com) (Nick Desaulniers clang version 12.0.0 (g...@github.com:llvm/llvm-project.git e75fec2b238f0e26cfb7645f2208baebe3440d41), GNU ld (GNU Binutils for Debian) 2.35.1) #76 SMP Thu Jan 7 17:51:34 PST 2021 / # uname -a Linux (none) 5.11.0-rc2-01914-g16586f130181-dirty #76 SMP Thu Jan 7 17:51:34 PST 2021 s390x GNU/Linux For the series: Tested-by: Nick Desaulniers > > v1 -> v2: > - Add 's390x/tcg: Don't ignore content in r0 when not specified via "b" or > "x"' > - Add 's390x/tcg: Ignore register content if b1/b2 is zero when handling > EXEUTE' > - "s390x/tcg: Fix ALGSI" > -- Fixup subject > - "s390x/tcg: Fix RISBHG" > -- Rephrase description, stating that it fixes clang-11 > > David Hildenbrand (4): > s390x/tcg: Fix ALGSI > s390x/tcg: Fix RISBHG > s390x/tcg: Only ignore content in r0 when specified via "b" or "x" > s390x/tcg: Ignore register content if b1/b2 is zero when handling > EXECUTE > > target/s390x/insn-data.def | 10 +- > target/s390x/mem_helper.c | 4 ++-- > target/s390x/translate.c | 33 + > 3 files changed, 24 insertions(+), 23 deletions(-) > > -- > 2.29.2 > -- Thanks, ~Nick Desaulniers
Re: [PATCH v1] s390x/tcg: Fix RISBHG
On Thu, Jan 7, 2021 at 3:27 PM David Hildenbrand wrote:
>
>
> > Am 08.01.2021 um 00:21 schrieb Nick Desaulniers :
> >
> > On Thu, Jan 7, 2021 at 3:13 PM David Hildenbrand wrote:
> >>
> >> RISBHG is broken and currently hinders clang builds of upstream kernels
> >> from booting: the kernel crashes early, while decompressing the image.
> >>
> >> [...]
> >> Kernel fault: interruption code 0005 ilc:2
> >> Kernel random base:
> >> PSW : 20018000 00017a1e
> >> R:0 T:0 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:2 PM:0 RI:0 EA:3
> >> GPRS: 0001 000c 0003fff4 fff0
> >> fff4 000c fff0
> >> fffc fff8 008e25a8
> >> 0009 0002 0008 bce0
> >>
> >> One example of a buggy instruction is:
> >>
> >>17dde: ec 1e 00 9f 20 5d risbhg %r1,%r14,0,159,32
> >>
> >> With %r14 = 0x9 and %r1 = 0x7 should result in %r1 = 0x90007, however,
> >> results in %r1 = 0.
> >>
> >> Let's interpret values of i3/i4 as documented in the PoP and make
> >> computation of "mask" only based on i3 and i4 and use "pmask" only at the
> >> very end to make sure wrapping is only applied to the high/low doubleword.
> >>
> >> With this patch, I can successfully boot a v5.10 kernel built with
> >> clang, and gcc builds keep on working.
> >>
> >> Fixes: 2d6a869833d9 ("target-s390: Implement RISBG")
> >> Reported-by: Nick Desaulniers
> >> Cc: Guenter Roeck
> >> Cc: Christian Borntraeger
> >> Signed-off-by: David Hildenbrand
> >> ---
> >>
> >> This BUG was a nightmare to debug and the code a nightmare to understand.
> >>
> >> To make clang/gcc builds boot, the following fix is required as well on
> >> top of current master: "[PATCH] target/s390x: Fix ALGSI"
> >> https://lkml.kernel.org/r/20210107202135.52379-1-da...@redhat.com
> >
> > In that case, a huge thank you!!! for this work! ++beers_owed.
> >
>
> :) a kernel build for z13 should work with the (default) „-cpu qemu“ cpu type.
Hmm...so I don't think clang can build a Linux kernel image with
CONFIG_MARCH_Z13=y just yet; just defconfig. Otherwise looks like
clang barfs on some of the inline asm constraints.
It looks like with your patch applied we get further into the boot!
I'm not seeing any output with:
$ /android0/qemu/build/qemu-system-s390x -cpu qemu -append
'conmode=sclp console=ttyS0' -display none -initrd
//boot-utils/images/s390/rootfs.cpio -kernel
arch/s390/boot/bzImage -m 512m -nodefaults -serial mon:stdio
(Based on a quick skim through
https://www.ibm.com/support/knowledgecenter/en/linuxonibm/com.ibm.linux.z.ludd/ludd_r_lmtkernelparameter.html).
Do I have all of those right?
If I attach GDB to QEMU running that kernel image, I was able to view
the print banner once via `lx-dmesg` gdb macro in the kernel, but it
seems on subsequent runs control flow gets diverted unexpected post
entry to start_kernel() always to `s390_base_pgm_handler` ...errr..at
least when I try to single step in GDB. Tried with linux-5.10.y,
mainline, and linux-next.
qemu: 470dd6bd360782f5137f7e3376af6a44658eb1d3 + your patch
llvm: 106e66f3f555c8f887e82c5f04c3e77bdaf345e8
linux-5.10.y: d1988041d19dc8b532579bdbb7c4a978391c0011
linux: 71c061d2443814de15e177489d5cc00a4a253ef3
linux-next: f87684f6470f5f02bd47d4afb900366e5d2f31b6
(gdb) hbreak setup_arch
Hardware assisted breakpoint 1 at 0x142229e: file
arch/s390/kernel/setup.c, line 1091.
(gdb) c
Continuing.
Program received signal SIGTRAP, Trace/breakpoint trap.
0x014222a0 in setup_arch (cmdline_p=0x11d7ed8) at
arch/s390/kernel/setup.c:1091
1091if (MACHINE_IS_VM)
(gdb) lx-dmesg
[0.376351] Linux version 5.11.0-rc2-00157-ga2885c701c30
(ndesaulni...@ndesaulniers1.mtv.corp.google.com) (Nick Desaulniers
clang version 12.0.0 (g...@github.com:llvm/llvm-project.git
e75fec2b238f0e26cfb7645f2208baebe3440d41), GNU ld (GNU Binutils for
Debian) 2.35.1) #81 SMP Thu Jan 7 17:57:34 PST 2021
>
> >>
> >> ---
> >> target/s390x/translate.c | 18 --
> >> 1 file changed, 8 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> >> index 3d5c0d6106..39e33eeb67 100644
> >> --- a/target/s390x/translate.c
> >> +++ b/target/s390x/translate.c
> >> @@ -3815,22
Re: [PATCH v1] s390x/tcg: Fix RISBHG
On Thu, Jan 7, 2021 at 3:13 PM David Hildenbrand wrote:
>
> RISBHG is broken and currently hinders clang builds of upstream kernels
> from booting: the kernel crashes early, while decompressing the image.
>
> [...]
>Kernel fault: interruption code 0005 ilc:2
>Kernel random base:
>PSW : 20018000 00017a1e
> R:0 T:0 IO:0 EX:0 Key:0 M:0 W:0 P:0 AS:0 CC:2 PM:0 RI:0 EA:3
>GPRS: 0001 000c 0003fff4 fff0
> fff4 000c fff0
> fffc fff8 008e25a8
> 0009 0002 0008 bce0
>
> One example of a buggy instruction is:
>
> 17dde: ec 1e 00 9f 20 5d risbhg %r1,%r14,0,159,32
>
> With %r14 = 0x9 and %r1 = 0x7 should result in %r1 = 0x90007, however,
> results in %r1 = 0.
>
> Let's interpret values of i3/i4 as documented in the PoP and make
> computation of "mask" only based on i3 and i4 and use "pmask" only at the
> very end to make sure wrapping is only applied to the high/low doubleword.
>
> With this patch, I can successfully boot a v5.10 kernel built with
> clang, and gcc builds keep on working.
>
> Fixes: 2d6a869833d9 ("target-s390: Implement RISBG")
> Reported-by: Nick Desaulniers
> Cc: Guenter Roeck
> Cc: Christian Borntraeger
> Signed-off-by: David Hildenbrand
> ---
>
> This BUG was a nightmare to debug and the code a nightmare to understand.
>
> To make clang/gcc builds boot, the following fix is required as well on
> top of current master: "[PATCH] target/s390x: Fix ALGSI"
> https://lkml.kernel.org/r/20210107202135.52379-1-da...@redhat.com
In that case, a huge thank you!!! for this work! ++beers_owed.
>
> ---
> target/s390x/translate.c | 18 --
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 3d5c0d6106..39e33eeb67 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3815,22 +3815,23 @@ static DisasJumpType op_risbg(DisasContext *s,
> DisasOps *o)
> pmask = 0xull;
> break;
> case 0x51: /* risblg */
> -i3 &= 31;
> -i4 &= 31;
> +i3 = (i3 & 31) + 32;
> +i4 = (i4 & 31) + 32;
> pmask = 0xull;
> break;
> default:
> g_assert_not_reached();
> }
>
> -/* MASK is the set of bits to be inserted from R2.
> - Take care for I3/I4 wraparound. */
> -mask = pmask >> i3;
> +/* MASK is the set of bits to be inserted from R2. */
> if (i3 <= i4) {
> -mask ^= pmask >> i4 >> 1;
> +/* [0...i3---i4...63] */
> +mask = (-1ull >> i3) & (-1ull << (63 - i4));
> } else {
> -mask |= ~(pmask >> i4 >> 1);
> +/* [0---i4...i3---63] */
> +mask = (-1ull >> i3) | (-1ull << (63 - i4));
> }
The expression evaluated looks the same to me for both sides of the
conditional, but the comments differ. Intentional?
> +/* For RISBLG/RISBHG, the wrapping is limited to the high/low
> doubleword. */
> mask &= pmask;
>
> /* IMASK is the set of bits to be kept from R1. In the case of the
> high/low
> @@ -3843,9 +3844,6 @@ static DisasJumpType op_risbg(DisasContext *s, DisasOps
> *o)
> len = i4 - i3 + 1;
> pos = 63 - i4;
> rot = i5 & 63;
> -if (s->fields.op2 == 0x5d) {
> -pos += 32;
> -}
>
> /* In some cases we can implement this with extract. */
> if (imask == 0 && pos == 0 && len > 0 && len <= rot) {
> --
> 2.29.2
>
--
Thanks,
~Nick Desaulniers
[Bug 1894869]
https://bugs.launchpad.net/qemu/+bug/1894869
Here's the discussion with the upstream devs. The problem ended up being
on Chelsio's part as either the .7 funciton fo these cards should not
have even been exposed to the OS in the first place, or SR-IOV is
necessary to actually correct the parameters of this function.
Unfortunately, it looks like SR-IOV is no longer possible to enable on
these cards. Thank you for your help.
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1894869
Title:
Chelsio T4 has old MSIX PBA offset bug
Status in QEMU:
Invalid
Status in Debian:
In Progress
Bug description:
There exists a bug with Chelsio NICs T4 that causes the following
error:
kvm: -device vfio-
pci,host=:83:00.7,id=hostpci1.7,bus=pci.0,addr=0x11.7: vfio
:83:00.7: hardware reports invalid configuration, MSIX PBA outside
of specified BAR
I discovered this bug on a Proxmox system, and I was working with a
downstream Proxmox developer to try to fix this issue. They provided
me with the following change to make from line 1484 of hw/vfio/pci.c:
static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
* is 0x1000, so we hard code that here.
*/
if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
-(vdev->device_id & 0xff00) == 0x5800) {
+((vdev->device_id & 0xff00) == 0x5800 ||
+ (vdev->device_id & 0xff00) == 0x1425)) {
msix->pba_offset = 0x1000;
} else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
error_setg(errp, "hardware reports invalid configuration, "
However, I found that this did not fix the issue, so the bug appears
to work differently than the one that was present on the T5 NICs which
has already been patched. I have attached the output of my lspci
-nnkvv
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1894869/+subscriptions
[Bug 1894869] Re: Chelsio T4 has old MSIX PBA offset bug
I was able to boot a VM with just the functions of the device with the
ethernet controller function ID added as PCI devices. Something I
noticed while adding in those devices though is that all of the others
have a description associated with them in Proxmox, but the one that's
causing the boot to fail doesn't. I attached a picture of the menu,
81:00.7 has no functions associated with it. So it seems like it just
doesn't have any function at all? Unless it benefits QEMU to know
whether turning SR-IOV on for these cards fixes the problem, I don't
think I'm going to go through the process of turning it on, since the
process looks terrible. Thank you for your help.
** Attachment added: "no 7th.JPG"
https://bugs.launchpad.net/qemu/+bug/1894869/+attachment/5411172/+files/no%207th.JPG
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1894869
Title:
Chelsio T4 has old MSIX PBA offset bug
Status in QEMU:
New
Status in Debian:
In Progress
Bug description:
There exists a bug with Chelsio NICs T4 that causes the following
error:
kvm: -device vfio-
pci,host=:83:00.7,id=hostpci1.7,bus=pci.0,addr=0x11.7: vfio
:83:00.7: hardware reports invalid configuration, MSIX PBA outside
of specified BAR
I discovered this bug on a Proxmox system, and I was working with a
downstream Proxmox developer to try to fix this issue. They provided
me with the following change to make from line 1484 of hw/vfio/pci.c:
static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
* is 0x1000, so we hard code that here.
*/
if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
-(vdev->device_id & 0xff00) == 0x5800) {
+((vdev->device_id & 0xff00) == 0x5800 ||
+ (vdev->device_id & 0xff00) == 0x1425)) {
msix->pba_offset = 0x1000;
} else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
error_setg(errp, "hardware reports invalid configuration, "
However, I found that this did not fix the issue, so the bug appears
to work differently than the one that was present on the T5 NICs which
has already been patched. I have attached the output of my lspci
-nnkvv
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1894869/+subscriptions
[Bug 1894869] Re: Chelsio T4 has old MSIX PBA offset bug
Yeah, I figured out that the logic behind that patch was failed and
corrected it to get the same error again already. Just to clarify, it is
two of the same card giving the same error. I ran dmesg --level=err, but
got no output. In the full output of dmesg, though, I noticed that there
are some problems with the nics, but I don't know enough about this to
know if there's anything I can do about it. I included dmesg output
here. I don't believe the nics are giving the host any functionality
since I added the driver for them to the blacklist, so it shouldn't even
be getting loaded by it. In case it's useful, I'm not sure if SR-IOV is
enabled on these cards or not, though I'm trying to use PCI passthrough
for my VMs.
** Attachment added: "Output of dmesg"
https://bugs.launchpad.net/qemu/+bug/1894869/+attachment/5410926/+files/dmesg.txt
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1894869
Title:
Chelsio T4 has old MSIX PBA offset bug
Status in QEMU:
New
Status in Debian:
In Progress
Bug description:
There exists a bug with Chelsio NICs T4 that causes the following
error:
kvm: -device vfio-
pci,host=:83:00.7,id=hostpci1.7,bus=pci.0,addr=0x11.7: vfio
:83:00.7: hardware reports invalid configuration, MSIX PBA outside
of specified BAR
I discovered this bug on a Proxmox system, and I was working with a
downstream Proxmox developer to try to fix this issue. They provided
me with the following change to make from line 1484 of hw/vfio/pci.c:
static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
* is 0x1000, so we hard code that here.
*/
if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
-(vdev->device_id & 0xff00) == 0x5800) {
+((vdev->device_id & 0xff00) == 0x5800 ||
+ (vdev->device_id & 0xff00) == 0x1425)) {
msix->pba_offset = 0x1000;
} else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
error_setg(errp, "hardware reports invalid configuration, "
However, I found that this did not fix the issue, so the bug appears
to work differently than the one that was present on the T5 NICs which
has already been patched. I have attached the output of my lspci
-nnkvv
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1894869/+subscriptions
[Bug 1894869] Re: Chelsio T4 has old MSIX PBA offset bug
** Bug watch added: bugzilla.proxmox.com/ #2969
https://bugzilla.proxmox.com/show_bug.cgi?id=2969
** Also affects: debian via
https://bugzilla.proxmox.com/show_bug.cgi?id=2969
Importance: Unknown
Status: Unknown
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1894869
Title:
Chelsio T4 has old MSIX PBA offset bug
Status in QEMU:
New
Status in Debian:
Unknown
Bug description:
There exists a bug with Chelsio NICs T4 that causes the following
error:
kvm: -device vfio-
pci,host=:83:00.7,id=hostpci1.7,bus=pci.0,addr=0x11.7: vfio
:83:00.7: hardware reports invalid configuration, MSIX PBA outside
of specified BAR
I discovered this bug on a Proxmox system, and I was working with a
downstream Proxmox developer to try to fix this issue. They provided
me with the following change to make from line 1484 of hw/vfio/pci.c:
static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
* is 0x1000, so we hard code that here.
*/
if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
-(vdev->device_id & 0xff00) == 0x5800) {
+((vdev->device_id & 0xff00) == 0x5800 ||
+ (vdev->device_id & 0xff00) == 0x1425)) {
msix->pba_offset = 0x1000;
} else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
error_setg(errp, "hardware reports invalid configuration, "
However, I found that this did not fix the issue, so the bug appears
to work differently than the one that was present on the T5 NICs which
has already been patched. I have attached the output of my lspci
-nnkvv
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1894869/+subscriptions
[Bug 1894869] Re: Chelsio T4 has old MSIX PBA offset bug
** Description changed:
There exists a bug with Chelsio NICs T4 that causes the following error:
kvm: -device vfio-
pci,host=:83:00.7,id=hostpci1.7,bus=pci.0,addr=0x11.7: vfio
:83:00.7: hardware reports invalid configuration, MSIX PBA outside
of specified BAR
- I was working with a downstream Proxmox developer to try to fix this
- issue, and they provided me with the following change to make from line
- 1484 of hw/vfio/pci.c:
+ I discovered this bug on a Proxmox system, and I was working with a
+ downstream Proxmox developer to try to fix this issue. They provided me
+ with the following change to make from line 1484 of hw/vfio/pci.c:
static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
- * is 0x1000, so we hard code that here.
- */
- if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
+ * is 0x1000, so we hard code that here.
+ */
+ if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
-(vdev->device_id & 0xff00) == 0x5800) {
+((vdev->device_id & 0xff00) == 0x5800 ||
+ (vdev->device_id & 0xff00) == 0x1425)) {
- msix->pba_offset = 0x1000;
- } else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
- error_setg(errp, "hardware reports invalid configuration, "
+ msix->pba_offset = 0x1000;
+ } else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
+ error_setg(errp, "hardware reports invalid configuration, "
However, I found that this did not fix the issue, so the bug appears to
work differently than the one that was present on the T5 NICs which has
already been patched. I have attached the output of my lspci -nnkvv
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1894869
Title:
Chelsio T4 has old MSIX PBA offset bug
Status in QEMU:
New
Bug description:
There exists a bug with Chelsio NICs T4 that causes the following
error:
kvm: -device vfio-
pci,host=:83:00.7,id=hostpci1.7,bus=pci.0,addr=0x11.7: vfio
:83:00.7: hardware reports invalid configuration, MSIX PBA outside
of specified BAR
I discovered this bug on a Proxmox system, and I was working with a
downstream Proxmox developer to try to fix this issue. They provided
me with the following change to make from line 1484 of hw/vfio/pci.c:
static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
* is 0x1000, so we hard code that here.
*/
if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
-(vdev->device_id & 0xff00) == 0x5800) {
+((vdev->device_id & 0xff00) == 0x5800 ||
+ (vdev->device_id & 0xff00) == 0x1425)) {
msix->pba_offset = 0x1000;
} else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
error_setg(errp, "hardware reports invalid configuration, "
However, I found that this did not fix the issue, so the bug appears
to work differently than the one that was present on the T5 NICs which
has already been patched. I have attached the output of my lspci
-nnkvv
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1894869/+subscriptions
[Bug 1894869] [NEW] Chelsio T4 has old MSIX PBA offset bug
Public bug reported:
There exists a bug with Chelsio NICs T4 that causes the following error:
kvm: -device vfio-
pci,host=:83:00.7,id=hostpci1.7,bus=pci.0,addr=0x11.7: vfio
:83:00.7: hardware reports invalid configuration, MSIX PBA outside
of specified BAR
I was working with a downstream Proxmox developer to try to fix this
issue, and they provided me with the following change to make from line
1484 of hw/vfio/pci.c:
static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
* is 0x1000, so we hard code that here.
*/
if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
-(vdev->device_id & 0xff00) == 0x5800) {
+((vdev->device_id & 0xff00) == 0x5800 ||
+ (vdev->device_id & 0xff00) == 0x1425)) {
msix->pba_offset = 0x1000;
} else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
error_setg(errp, "hardware reports invalid configuration, "
However, I found that this did not fix the issue, so the bug appears to
work differently than the one that was present on the T5 NICs which has
already been patched. I have attached the output of my lspci -nnkvv
** Affects: qemu
Importance: Undecided
Status: New
** Tags: chelsio t4
** Attachment added: "Full lspci -nnkvv output"
https://bugs.launchpad.net/bugs/1894869/+attachment/5408718/+files/lspci.txt
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1894869
Title:
Chelsio T4 has old MSIX PBA offset bug
Status in QEMU:
New
Bug description:
There exists a bug with Chelsio NICs T4 that causes the following
error:
kvm: -device vfio-
pci,host=:83:00.7,id=hostpci1.7,bus=pci.0,addr=0x11.7: vfio
:83:00.7: hardware reports invalid configuration, MSIX PBA outside
of specified BAR
I was working with a downstream Proxmox developer to try to fix this
issue, and they provided me with the following change to make from
line 1484 of hw/vfio/pci.c:
static void vfio_msix_early_setup(VFIOPCIDevice *vdev, Error **errp)
* is 0x1000, so we hard code that here.
*/
if (vdev->vendor_id == PCI_VENDOR_ID_CHELSIO &&
-(vdev->device_id & 0xff00) == 0x5800) {
+((vdev->device_id & 0xff00) == 0x5800 ||
+ (vdev->device_id & 0xff00) == 0x1425)) {
msix->pba_offset = 0x1000;
} else if (vdev->msix_relo == OFF_AUTOPCIBAR_OFF) {
error_setg(errp, "hardware reports invalid configuration, "
However, I found that this did not fix the issue, so the bug appears
to work differently than the one that was present on the T5 NICs which
has already been patched. I have attached the output of my lspci
-nnkvv
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1894869/+subscriptions
[PATCH] Provide a NetBSD specific aarch64 cpu_signal_handler
Fix qemu build on NetBSD/evbarm-aarch64 by providing a NetBSD specific
cpu_signal_handler.
Signed-off-by: Nick Hudson
---
accel/tcg/user-exec.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 4be78eb9b3..dd128adc00 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -523,6 +523,31 @@ int cpu_signal_handler(int host_signum, void *pinfo,
#elif defined(__aarch64__)
+#if defined(__NetBSD__)
+
+#include
+#include
+
+int cpu_signal_handler(int host_signum, void *pinfo, void *puc)
+{
+ucontext_t *uc = puc;
+siginfo_t *si = pinfo;
+unsigned long pc;
+int is_write;
+uint32_t esr;
+
+pc = uc->uc_mcontext.__gregs[_REG_PC];
+esr = si->si_trap;
+
+/* siginfo_t::si_trap is the ESR value, for data aborts ESR.EC
+ * is 0b10010x: then bit 6 is the WnR bit
+ */
+is_write = extract32(esr, 27, 5) == 0x12 && extract32(esr, 6, 1) == 1;
+return handle_cpu_signal(pc, si, is_write, &uc->uc_sigmask);
+}
+
+#else
+
#ifndef ESR_MAGIC
/* Pre-3.16 kernel headers don't have these, so provide fallback definitions */
#define ESR_MAGIC 0x45535201
@@ -585,6 +610,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, void
*puc)
}
return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
}
+#endif
#elif defined(__s390__)
--
2.17.1
[PATCH] Provide a NetBSD specific aarch64 cpu_signal_handler
Fix qemu build on NetBSD/evbarm-aarch64 by providing a NetBSD specific
cpu_signal_handler.
Signed-off-by: Nick Hudson
---
accel/tcg/user-exec.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 4be78eb9b3..dd128adc00 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -523,6 +523,31 @@ int cpu_signal_handler(int host_signum, void *pinfo,
#elif defined(__aarch64__)
+#if defined(__NetBSD__)
+
+#include
+#include
+
+int cpu_signal_handler(int host_signum, void *pinfo, void *puc)
+{
+ucontext_t *uc = puc;
+siginfo_t *si = pinfo;
+unsigned long pc;
+int is_write;
+uint32_t esr;
+
+pc = uc->uc_mcontext.__gregs[_REG_PC];
+esr = si->si_trap;
+
+/* siginfo_t::si_trap is the ESR value, for data aborts ESR.EC
+ * is 0b10010x: then bit 6 is the WnR bit
+ */
+is_write = extract32(esr, 27, 5) == 0x12 && extract32(esr, 6, 1) == 1;
+return handle_cpu_signal(pc, si, is_write, &uc->uc_sigmask);
+}
+
+#else
+
#ifndef ESR_MAGIC
/* Pre-3.16 kernel headers don't have these, so provide fallback definitions */
#define ESR_MAGIC 0x45535201
@@ -585,6 +610,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, void
*puc)
}
return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask);
}
+#endif
#elif defined(__s390__)
--
2.17.1
[PATCH v2] NetBSD/arm build fix
Fix building on NetBSD/arm by extracting the FSR value from the correct siginfo_t field. Signed-off-by: Nick Hudson --- accel/tcg/user-exec.c | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index 52359949df..bc391eb454 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -517,6 +517,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, #if defined(__NetBSD__) #include +#include #endif int cpu_signal_handler(int host_signum, void *pinfo, @@ -525,10 +526,12 @@ int cpu_signal_handler(int host_signum, void *pinfo, siginfo_t *info = pinfo; #if defined(__NetBSD__) ucontext_t *uc = puc; +siginfo_t *si = pinfo; #else ucontext_t *uc = puc; #endif unsigned long pc; +uint32_t fsr; int is_write; #if defined(__NetBSD__) @@ -539,10 +542,17 @@ int cpu_signal_handler(int host_signum, void *pinfo, pc = uc->uc_mcontext.arm_pc; #endif -/* error_code is the FSR value, in which bit 11 is WnR (assuming a v6 or - * later processor; on v5 we will always report this as a read). +#ifdef __NetBSD__ +fsr = si->si_trap; +#else +fsr = uc->uc_mcontext.error_code; +#endif +/* + * In the FSR, bit 11 is WnR, assuming a v6 or + * later processor. On v5 we will always report + * this as a read, which will fail later. */ -is_write = extract32(uc->uc_mcontext.error_code, 11, 1); +is_write = extract32(fsr, 11, 1); return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); } -- 2.17.1
[PATCH 1/1] NetBSD/arm build fix
Fix building on NetBSD/arm by extracting the FSR value from the correct siginfo_t field. Signed-off-by: Nick Hudson --- accel/tcg/user-exec.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c index 52359949df..3637626456 100644 --- a/accel/tcg/user-exec.c +++ b/accel/tcg/user-exec.c @@ -517,6 +517,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, #if defined(__NetBSD__) #include +#include #endif int cpu_signal_handler(int host_signum, void *pinfo, @@ -525,6 +526,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, siginfo_t *info = pinfo; #if defined(__NetBSD__) ucontext_t *uc = puc; +siginfo_t *si = pinfo; #else ucontext_t *uc = puc; #endif @@ -539,10 +541,18 @@ int cpu_signal_handler(int host_signum, void *pinfo, pc = uc->uc_mcontext.arm_pc; #endif +#if defined(__NetBSD__) +/* siginfo_t::si_trap is the FSR value, in which bit 11 is WnR + * (assuming a v6 or later processor; on v5 we will always report + * this as a read). + */ +is_write = extract32(si->si_trap, 11, 1); +#else /* error_code is the FSR value, in which bit 11 is WnR (assuming a v6 or * later processor; on v5 we will always report this as a read). */ is_write = extract32(uc->uc_mcontext.error_code, 11, 1); +#endif return handle_cpu_signal(pc, info, is_write, &uc->uc_sigmask); } -- 2.17.1
[PATCH] vhost-vsock: fix error message output
error_setg_errno takes a positive error number, so we should not invert
errno's sign.
Signed-off-by: Nick Erdmann
---
hw/virtio/vhost-vsock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index 66da96583b..9f9093e196 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -325,7 +325,7 @@ static void vhost_vsock_device_realize(DeviceState *dev,
Error **errp)
} else {
vhostfd = open("/dev/vhost-vsock", O_RDWR);
if (vhostfd < 0) {
-error_setg_errno(errp, -errno,
+error_setg_errno(errp, errno,
"vhost-vsock: failed to open vhost device");
return;
}
--
2.25.1
[Bug 1865099] [NEW] cannot run x64 based system on x64 host with Intel Haxm
Public bug reported: i am trying to run Windows 10 x64 on Windows 10 x64 host with intel haxm as kernel accelerator, but the system never boots, as far i read the documentation everything should be fine... the logs are qemu: ` D:\vm>qemu-system-x86_64 -d guest_errors,out_asm,in_asm,op,op_opt,op_ind,int,exec,cpu,fpu,mmu,pcall,cpu_reset,unimp,page,nochain -cpu core2duo -smp 4 -accel hax -drive file=w10.img,index=0,media=disk,format=raw -cdrom "E:\test\W10x64ProEn-UK.iso" -m 4G -L Bios -usbdevice mouse -usbdevice keyboard -boot menu=on -rtc base=localtime,clock=host -name windows qemu-system-x86_64: -usbdevice mouse: '-usbdevice' is deprecated, please use '-device usb-...' instead qemu-system-x86_64: -usbdevice keyboard: '-usbdevice' is deprecated, please use '-device usb-...' instead HAX is working and emulator runs in fast virt mode. CPU Reset (CPU 0) EAX= EBX= ECX= EDX= ESI= EDI= EBP= ESP= EIP= EFL= [---] CPL=0 II=0 A20=0 SMM=0 HLT=0 ES = CS = SS = DS = FS = GS = LDT= TR = GDT= IDT= CR0= CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6= DR7= CCS= CCD= CCO=DYNAMIC EFER= FCW= FSW= [ST=0] FTW=ff MXCSR= FPR0= FPR1= FPR2= FPR3= FPR4= FPR5= FPR6= FPR7= XMM00= XMM01= XMM02= XMM03= XMM04= XMM05= XMM06= XMM07= CR0 update: CR0=0x6010 CPU Reset (CPU 1) EAX= EBX= ECX= EDX= ESI= EDI= EBP= ESP= EIP= EFL= [---] CPL=0 II=0 A20=0 SMM=0 HLT=0 ES = CS = SS = DS = FS = GS = LDT= TR = GDT= IDT= CR0= CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6= DR7= CCS= CCD= CCO=DYNAMIC EFER= FCW= FSW= [ST=0] FTW=ff MXCSR= FPR0= FPR1= FPR2= FPR3= FPR4= FPR5= FPR6= FPR7= XMM00= XMM01= XMM02= XMM03= XMM04= XMM05= XMM06= XMM07= CR0 update: CR0=0x6010 CPU Reset (CPU 2) EAX= EBX= ECX= EDX= ESI= EDI= EBP= ESP= EIP= EFL= [---] CPL=0 II=0 A20=0 SMM=0 HLT=0 ES = CS = SS = DS = FS = GS = LDT= TR = GDT= IDT= CR0= CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6= DR7= CCS= CCD= CCO=DYNAMIC EFER= FCW= FSW= [ST=0] FTW=ff MXCSR= FPR0= FPR1= FPR2= FPR3= FPR4= FPR5= FPR6= FPR7= XMM00= XMM01= XMM02= XMM03= XMM04= XMM05= XMM06= XMM07= CR0 update: CR0=0x6010 CPU Reset (CPU 3) EAX= EB
[Qemu-devel] [PATCH v3 2/2] Always return EXCP_DMAR for protection id trap as EXCP_DMP is considered legacy.
From: Nick Hudson
"In PA-RISC 1.1 (Second Edition) and later revisions, processors must use
traps 26, 27,and 28 which provide equivalent functionality"
Signed-off-by: Nick Hudson
---
target/hppa/mem_helper.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index c9b57d07c3..77fb544838 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -154,8 +154,7 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr
addr, int mmu_idx,
if (unlikely(!(prot & type))) {
/* The access isn't allowed -- Inst/Data Memory Protection Fault. */
-ret = (type & PAGE_EXEC ? EXCP_IMP :
- prot & PAGE_READ ? EXCP_DMP : EXCP_DMAR);
+ret = (type & PAGE_EXEC) ? EXCP_IMP : EXCP_DMAR;
goto egress;
}
--
2.17.1
[Qemu-devel] [PATCH v3 1/2] Implement the pcxl and pcxl2 Fast TLB Insert instructions as used by NetBSD (and OpenBSD)
From: Nick Hudson
See
https://parisc.wiki.kernel.org/images-parisc/a/a9/Pcxl2_ers.pdf
page 13-9 (195/206)
Signed-off-by: Nick Hudson
---
target/hppa/insns.decode | 3 +++
target/hppa/translate.c | 52
2 files changed, 55 insertions(+)
diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 098370c2f0..f0dd71dd08 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -133,6 +133,9 @@ ixtlbx 01 b:5 r:5 sp:2 010 addr:1 0 0
data=1
ixtlbx 01 b:5 r:5 ... 00 addr:1 0 0\
sp=%assemble_sr3x data=0
+# pcxl and pcxl2 Fast TLB Insert instructions
+ixtlbxf 01 0 r:5 00 0 data:1 01000 addr:1 0 0
+
pxtlbx 01 b:5 x:5 sp:2 0100100 local:1 m:1 - data=1
pxtlbx 01 b:5 x:5 ... 000100 local:1 m:1 - \
sp=%assemble_sr3x data=0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 43b74367ea..8d59990cfe 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2518,6 +2518,58 @@ static bool trans_pxtlbx(DisasContext *ctx, arg_pxtlbx
*a)
#endif
}
+/* Implement the pcxl and pcxl2 Fast TLB Insert instructions.
+ * See
+ * https://parisc.wiki.kernel.org/images-parisc/a/a9/Pcxl2_ers.pdf
+ * page 13-9 (195/206) */
+static bool trans_ixtlbxf(DisasContext *ctx, arg_ixtlbxf *a)
+{
+CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
+#ifndef CONFIG_USER_ONLY
+TCGv_tl addr;
+TCGv_reg reg;
+TCGv_reg ar, sr;
+TCGv_tl atl, stl;
+
+nullify_over(ctx);
+
+/*if (not (pcxl or pcxl2))
+ return gen_illegal(ctx); */
+
+ar = get_temp(ctx);
+sr = get_temp(ctx);
+atl = get_temp_tl(ctx);
+stl = get_temp_tl(ctx);
+addr = get_temp_tl(ctx);
+
+
+if (a->data) {
+tcg_gen_ld_reg(sr, cpu_env, offsetof(CPUHPPAState, cr[CR_ISR]));
+tcg_gen_ld_reg(ar, cpu_env, offsetof(CPUHPPAState, cr[CR_IOR]));
+} else {
+tcg_gen_ld_reg(sr, cpu_env, offsetof(CPUHPPAState, cr[CR_IIASQ]));
+tcg_gen_ld_reg(ar, cpu_env, offsetof(CPUHPPAState, cr[CR_IIAOQ]));
+}
+
+tcg_gen_extu_reg_tl(atl, ar);
+tcg_gen_extu_reg_tl(stl, sr);
+tcg_gen_shli_i64(stl, stl, 32);
+tcg_gen_or_tl(addr, atl, stl);
+reg = load_gpr(ctx, a->r);
+if (a->addr) {
+gen_helper_itlba(cpu_env, addr, reg);
+} else {
+gen_helper_itlbp(cpu_env, addr, reg);
+}
+
+/* Exit TB for TLB change if mmu is enabled. */
+if (ctx->tb_flags & PSW_C) {
+ctx->base.is_jmp = DISAS_IAQ_N_STALE;
+}
+return nullify_end(ctx);
+#endif
+}
+
static bool trans_lpa(DisasContext *ctx, arg_ldst *a)
{
CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
--
2.17.1
[Qemu-devel] [PATCH v3 0/2] HPPA fixes for NetBSD/hppa emulation
v3 changes: - Don't use C99 comments and fix a typo in the comment v2 changes: - remove old debug code *** BLURB HERE *** Nick Hudson (2): Implement the pcxl and pcxl2 Fast TLB Insert instructions as used by NetBSD (and OpenBSD) Always return EXCP_DMAR for protection id trap as EXCP_DMP is considered legacy. target/hppa/insns.decode | 3 +++ target/hppa/mem_helper.c | 3 +-- target/hppa/translate.c | 52 3 files changed, 56 insertions(+), 2 deletions(-) -- 2.17.1
[Qemu-devel] [PATCH v2 2/2] Always return EXCP_DMAR for protection id trap as EXCP_DMP is considered legacy.
From: Nick Hudson
"In PA-RISC 1.1 (Second Edition) and later revisions, processors must use
traps 26, 27,and 28 which provide equivalent functionality"
Signed-off-by: Nick Hudson
---
target/hppa/mem_helper.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index c9b57d07c3..77fb544838 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -154,8 +154,7 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr
addr, int mmu_idx,
if (unlikely(!(prot & type))) {
/* The access isn't allowed -- Inst/Data Memory Protection Fault. */
-ret = (type & PAGE_EXEC ? EXCP_IMP :
- prot & PAGE_READ ? EXCP_DMP : EXCP_DMAR);
+ret = (type & PAGE_EXEC) ? EXCP_IMP : EXCP_DMAR;
goto egress;
}
--
2.17.1
[Qemu-devel] [PATCH v2 0/2] HPPA fixes for NetBSD/hppa emulation
From: Nick Hudson Here are the required changes to allow qemu to emulate NetBSD/hppa. v2 changes: - remove old debug code Nick Hudson (2): Implement the pcxl and pcxl2 Fast TLB Insert instructions as used by NetBSD (and OpenBSD) Always return EXCP_DMAR for protection id trap as EXCP_DMP is considered legacy. target/hppa/insns.decode | 3 +++ target/hppa/mem_helper.c | 3 +-- target/hppa/translate.c | 52 3 files changed, 56 insertions(+), 2 deletions(-) -- 2.17.1
[Qemu-devel] [PATCH v2 1/2] Implement the pcxl and pcxl2 Fast TLB Insert instructions as used by NetBSD (and OpenBSD)
From: Nick Hudson
See
https://parisc.wiki.kernel.org/images-parisc/a/a9/Pcxl2_ers.pdf
page 13-9 (195/206)
Signed-off-by: Nick Hudson
---
target/hppa/insns.decode | 3 +++
target/hppa/translate.c | 52
2 files changed, 55 insertions(+)
diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 098370c2f0..f0dd71dd08 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -133,6 +133,9 @@ ixtlbx 01 b:5 r:5 sp:2 010 addr:1 0 0
data=1
ixtlbx 01 b:5 r:5 ... 00 addr:1 0 0\
sp=%assemble_sr3x data=0
+# pcxl and pcxl2 Fast TLB Insert instructions
+ixtlbxf 01 0 r:5 00 0 data:1 01000 addr:1 0 0
+
pxtlbx 01 b:5 x:5 sp:2 0100100 local:1 m:1 - data=1
pxtlbx 01 b:5 x:5 ... 000100 local:1 m:1 - \
sp=%assemble_sr3x data=0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 43b74367ea..6038cbc3dd 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2518,6 +2518,58 @@ static bool trans_pxtlbx(DisasContext *ctx, arg_pxtlbx
*a)
#endif
}
+/* Implement the pcxl and pcxl2 Fast TLB Insert instructions.
+ * See
+ * https://parisc.wiki.kernel.org/images-parisc/a/a9/Pcxl2_ers.pdf
+ * page 13-9 (195/206) */
+static bool trans_ixtlbxf(DisasContext *ctx, arg_ixtlbxf *a)
+{
+CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
+#ifndef CONFIG_USER_ONLY
+TCGv_tl addr;
+TCGv_reg reg;
+TCGv_reg ar, sr;
+TCGv_tl atl, stl;
+
+nullify_over(ctx);
+
+//if (not (pcx or pcxl2))
+//return gen_illegal(ctx);
+
+ar = get_temp(ctx);
+sr = get_temp(ctx);
+atl = get_temp_tl(ctx);
+stl = get_temp_tl(ctx);
+addr = get_temp_tl(ctx);
+
+
+if (a->data) {
+tcg_gen_ld_reg(sr, cpu_env, offsetof(CPUHPPAState, cr[CR_ISR]));
+tcg_gen_ld_reg(ar, cpu_env, offsetof(CPUHPPAState, cr[CR_IOR]));
+} else {
+tcg_gen_ld_reg(sr, cpu_env, offsetof(CPUHPPAState, cr[CR_IIASQ]));
+tcg_gen_ld_reg(ar, cpu_env, offsetof(CPUHPPAState, cr[CR_IIAOQ]));
+}
+
+tcg_gen_extu_reg_tl(atl, ar);
+tcg_gen_extu_reg_tl(stl, sr);
+tcg_gen_shli_i64(stl, stl, 32);
+tcg_gen_or_tl(addr, atl, stl);
+reg = load_gpr(ctx, a->r);
+if (a->addr) {
+gen_helper_itlba(cpu_env, addr, reg);
+} else {
+gen_helper_itlbp(cpu_env, addr, reg);
+}
+
+/* Exit TB for TLB change if mmu is enabled. */
+if (ctx->tb_flags & PSW_C) {
+ctx->base.is_jmp = DISAS_IAQ_N_STALE;
+}
+return nullify_end(ctx);
+#endif
+}
+
static bool trans_lpa(DisasContext *ctx, arg_ldst *a)
{
CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
--
2.17.1
[Qemu-devel] [PATCH v1 1/2] Implement the pcxl and pcxl2 Fast TLB Insert, instructions as used by NetBSD (and OpenBSD)
Implement the pcxl and pcxl2 Fast TLB Insert instructions as used by NetBSD
(and OpenBSD)
See
https://parisc.wiki.kernel.org/images-parisc/a/a9/Pcxl2_ers.pdf
page 13-9 (195/206)
Signed-off-by: Nick Hudson
---
target/hppa/insns.decode | 3 +++
target/hppa/translate.c | 54
2 files changed, 57 insertions(+)
diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 098370c2f0..f0dd71dd08 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -133,6 +133,9 @@ ixtlbx 01 b:5 r:5 sp:2 010 addr:1 0 0
data=1
ixtlbx 01 b:5 r:5 ... 00 addr:1 0 0\
sp=%assemble_sr3x data=0
+# pcxl and pcxl2 Fast TLB Insert instructions
+ixtlbxf 01 0 r:5 00 0 data:1 01000 addr:1 0 0
+
pxtlbx 01 b:5 x:5 sp:2 0100100 local:1 m:1 - data=1
pxtlbx 01 b:5 x:5 ... 000100 local:1 m:1 - \
sp=%assemble_sr3x data=0
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 43b74367ea..860a659818 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2518,6 +2518,60 @@ static bool trans_pxtlbx(DisasContext *ctx, arg_pxtlbx
*a)
#endif
}
+/* Implement the pcxl and pcxl2 Fast TLB Insert instructions.
+ * See
+ * https://parisc.wiki.kernel.org/images-parisc/a/a9/Pcxl2_ers.pdf
+ * page 13-9 (195/206) */
+static bool trans_ixtlbxf(DisasContext *ctx, arg_ixtlbxf *a)
+{
+CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
+#ifndef CONFIG_USER_ONLY
+TCGv_tl addr;
+TCGv_reg reg;
+TCGv_reg ar, sr;
+TCGv_tl atl, stl;
+
+nullify_over(ctx);
+
+//if (not (pcx or pcxl2))
+//return gen_illegal(ctx);
+
+ar = get_temp(ctx);
+sr = get_temp(ctx);
+atl = get_temp_tl(ctx);
+stl = get_temp_tl(ctx);
+addr = get_temp_tl(ctx);
+
+
+if (a->data) {
+gen_helper_ixtlbxf_d(cpu_env);
+tcg_gen_ld_reg(sr, cpu_env, offsetof(CPUHPPAState, cr[CR_ISR]));
+tcg_gen_ld_reg(ar, cpu_env, offsetof(CPUHPPAState, cr[CR_IOR]));
+} else {
+gen_helper_ixtlbxf_i(cpu_env);
+tcg_gen_ld_reg(sr, cpu_env, offsetof(CPUHPPAState, cr[CR_IIASQ]));
+tcg_gen_ld_reg(ar, cpu_env, offsetof(CPUHPPAState, cr[CR_IIAOQ]));
+}
+
+tcg_gen_extu_reg_tl(atl, ar);
+tcg_gen_extu_reg_tl(stl, sr);
+tcg_gen_shli_i64(stl, stl, 32);
+tcg_gen_or_tl(addr, atl, stl);
+reg = load_gpr(ctx, a->r);
+if (a->addr) {
+gen_helper_itlba(cpu_env, addr, reg);
+} else {
+gen_helper_itlbp(cpu_env, addr, reg);
+}
+
+/* Exit TB for TLB change if mmu is enabled. */
+if (ctx->tb_flags & PSW_C) {
+ctx->base.is_jmp = DISAS_IAQ_N_STALE;
+}
+return nullify_end(ctx);
+#endif
+}
+
static bool trans_lpa(DisasContext *ctx, arg_ldst *a)
{
CHECK_MOST_PRIVILEGED(EXCP_PRIV_OPR);
--
2.17.1
[Qemu-devel] [PATCH v1 2/2] Always return EXCP_DMAR for protection id trap as, EXCP_DMP is considered legacy.
Always return EXCP_DMAR for protection id trap as EXCP_DMP is considered legacy.
"In PA-RISC 1.1 (Second Edition) and later revisions, processors must
use traps 26, 27,and 28 which provide equivalent functionality"
Signed-off-by: Nick Hudson
---
target/hppa/mem_helper.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index c9b57d07c3..77fb544838 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -154,8 +154,7 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr
addr, int mmu_idx,
if (unlikely(!(prot & type))) {
/* The access isn't allowed -- Inst/Data Memory Protection Fault. */
-ret = (type & PAGE_EXEC ? EXCP_IMP :
- prot & PAGE_READ ? EXCP_DMP : EXCP_DMAR);
+ret = (type & PAGE_EXEC) ? EXCP_IMP : EXCP_DMAR;
goto egress;
}
-- 2.17.1
[Qemu-devel] [PATCH v1 0/2] HPPA fixes for NetBSD/hppa emulation
Hi, Here are the required changes to allow qemu to emulate NetBSD/hppa. Nick Hudson (2): Implement the pcxl and pcxl2 Fast TLB Insert instructions as used by NetBSD (and OpenBSD) Always return EXCP_DMAR for protection id trap as EXCP_DMP is considered legacy. target/hppa/insns.decode | 3 +++ target/hppa/mem_helper.c | 3 +-- target/hppa/translate.c | 54 3 files changed, 58 insertions(+), 2 deletions(-) -- 2.17.1
Re: [Qemu-devel] ssh session with qemu-arm using busybox
Στις 2019-03-11 16:34, Pintu Agarwal έγραψε: Hi, I have a qemu-arm setup with busybox which I normally use to test my kernel changes. I use this command to boot it: QEMU-ARM$ qemu-system-arm -M vexpress-a9 -m 1024M -kernel ../KERNEL/linux/arch/arm/boot/zImage -dtb ../KERNEL/linux/arch/arm/boot/dts/vexpress-v2p-ca9.dtb -initrd rootfs.img.gz -append "console=ttyAMA0 root=/dev/ram rdinit=/sbin/init ip=dhcp" -nographic -smp 4 This includes, my own custom kernel and rootfs build. For rootfs I use busybox and create cpio image and use it as initrd. But, every time, if I have to copy some files inside qemu, I need to create create roofs image. This is painful. So, now I am exploring how to transfer files from my ubuntu pc to qemu-session using "ssh". I know this is possible, but I am still not successful, and bit lazy to explore these. I am sure, many of you people would have explored already "how to use ssh over qemu" and found a easy method. So, if anybody have easy setup please share with me. I could see that after adding "ip=dhcp" I get the eth0 interface like this: / # ifconfig eth0 Link encap:Ethernet HWaddr 52:54:00:12:34:56 inet addr:10.0.2.15 Bcast:10.0.2.255 Mask:255.255.255.0 UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1 RX packets:2 errors:0 dropped:0 overruns:0 frame:0 TX packets:2 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:1180 (1.1 KiB) TX bytes:1180 (1.1 KiB) Interrupt:22 loLink encap:Local Loopback inet addr:127.0.0.1 Mask:255.0.0.0 UP LOOPBACK RUNNING MTU:65536 Metric:1 RX packets:0 errors:0 dropped:0 overruns:0 frame:0 TX packets:0 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:0 (0.0 B) TX bytes:0 (0.0 B) But I could not ping it from ubuntu PC. Regards, Pintu ___ linux-riscv mailing list linux-ri...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv You may use the hostfw argument on qemu, e.g... -netdev user,id=unet,hostfwd=tcp::-:22 \ -net user \ and you 'll get guest's port 22 to be forwarded to hosts port , so you can do ssh root@localhost: from the host. Regards, Nick
Re: [Qemu-devel] AVX support for TCG
Thanks Stefan, Just a heads up, I don't know if I want to go for this myself yet. I'm still not sure I'll even be able to deliver the refactoring in time (Step 2 in your list), and I hadn't even considered tests for SSE as something I'd have to do since they're already implemented. In addition, there's been talks with a different project that I might apply for. I'll keep in touch if that changes, of course. Cheers, Nick Renieris. Στις Πέμ, 17 Ιαν 2019 στις 11:29 π.μ., ο/η Stefan Hajnoczi έγραψε: > > On Wed, Dec 26, 2018 at 1:29 AM Nick Renieris wrote: > > Do you think this could work as a GSoC project? I'm potentially > > interested in working on it this summer. > > Richard and Nick, > You are welcome to create a GSoC project idea and post it here: > https://wiki.qemu.org/Google_Summer_of_Code_2019 > > Good project ideas have the following characteristics: > * Well-defined - the scope is clear > * Self-contained - there are few dependencies > * Uncontroversial - they are acceptable to the community > * Incremental - they produce deliverables along the way > > Can you break down the project into tasks? > 1. Test cases for SSE and AVX > 2. SSE refactoring that needs to be done first > 3. AVX instructions that will be covered > 4. Stretch goals (AVX2, AVX512, etc) > > The project idea template looks like this: > > === TITLE === > > '''Summary:''' Short description of the project > > Detailed description of the project. > > '''Links:''' > * Wiki links to relevant material > * External links to mailing lists or web sites > > '''Details:''' > * Skill level: beginner or intermediate or advanced > * Language: C > * Mentor: Email address and IRC nick > * Suggested by: Person who suggested the idea > > > Note that project ideas are open to all applicants in order to ensure > fairness. > > Stefan
[Qemu-devel] [PATCH v5] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately.
The default location for the uboot file is 32MiB above bottom of DRAM.
This matches the recommendation in Documentation/arm/Booting.
Clarify the load_uimage API to state the passing of a load address when an
image doesn't specify one, or when loading a ramdisk is expected.
Adjust callers of load_uimage, etc.
Signed-off-by: Nick Hudson
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 19 ---
hw/core/uboot_image.h | 1 +
hw/microblaze/boot.c | 2 +-
hw/nios2/boot.c| 2 +-
hw/ppc/e500.c | 1 +
hw/ppc/ppc440_bamboo.c | 2 +-
hw/ppc/sam460ex.c | 2 +-
include/hw/loader.h| 7 ++-
9 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 94fce12802..c7a67af7a9 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x0200
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET8
@@ -1082,7 +1083,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
}
entry = elf_entry;
if (kernel_size < 0) {
-kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
+uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+kernel_size = load_uimage_as(info->kernel_filename, &entry, &loadaddr,
&is_linux, NULL, NULL, as);
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index fa41842280..c7182dfa64 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -613,13 +613,26 @@ static int load_uboot_image(const char *filename, hwaddr
*ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
-fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
-image_type);
-goto out;
+if (!(image_type == IH_TYPE_KERNEL &&
+hdr->ih_type == IH_TYPE_KERNEL_NOLOAD)) {
+fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
+image_type);
+goto out;
+}
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+case IH_TYPE_KERNEL_NOLOAD:
+if (!loadaddr || *loadaddr == LOAD_UIMAGE_LOADADDR_INVALID) {
+fprintf(stderr, "this image format (kernel_noload) cannot be "
+"loaded on this machine type");
+goto out;
+}
+
+hdr->ih_load = *loadaddr + sizeof(*hdr);
+hdr->ih_ep += hdr->ih_load;
+/* fall through */
case IH_TYPE_KERNEL:
address = hdr->ih_load;
if (translate_fn) {
diff --git a/hw/core/uboot_image.h b/hw/core/uboot_image.h
index 34c11a70a6..608022de6e 100644
--- a/hw/core/uboot_image.h
+++ b/hw/core/uboot_image.h
@@ -124,6 +124,7 @@
#define IH_TYPE_SCRIPT 6 /* Script file */
#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image (any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat Device Tree Blob */
+#define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image (noload) */
/*
* Compression Types
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 35bfeda7aa..489ab839b7 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -156,7 +156,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr
ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
-hwaddr uentry, loadaddr;
+hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, 0,
NULL, NULL);
diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index 4bb5b601d3..ed5cb28e94 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -161,7 +161,7 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
-hwaddr uentry, loadaddr;
+hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, 0,
Re: [Qemu-devel] [PATCH v4] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
Thanks for the comments.
On 07/01/2019 00:33, BALATON Zoltan wrote:
On Sun, 6 Jan 2019, Nick Hudson wrote:
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately.
The default location for the uboot file is 32MiB above bottom of DRAM.
This matches the recommendation in Documentation/arm/Booting.
Clarify the load_uimage API to state the passing of a load address
when an
image doesn't specify one, or when loading a ramdisk is expected.
Adjust callers of load_uimage, etc.
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 19 ---
hw/core/uboot_image.h | 1 +
hw/microblaze/boot.c | 2 +-
hw/nios2/boot.c | 2 +-
hw/ppc/e500.c | 1 +
hw/ppc/ppc440_bamboo.c | 2 +-
hw/ppc/sam460ex.c | 2 +-
include/hw/loader.h | 7 ++-
9 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 94fce12802..c7a67af7a9 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x0200
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET 8
@@ -1082,7 +1083,8 @@ void arm_load_kernel(ARMCPU *cpu, struct
arm_boot_info *info)
}
entry = elf_entry;
if (kernel_size < 0) {
- kernel_size = load_uimage_as(info->kernel_filename, &entry,
NULL,
+ uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+ kernel_size = load_uimage_as(info->kernel_filename, &entry,
&loadaddr,
&is_linux, NULL, NULL, as);
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index fa41842280..c7182dfa64 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -613,13 +613,26 @@ static int load_uboot_image(const char
*filename, hwaddr *ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
- fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
- image_type);
- goto out;
+ if (!(image_type == IH_TYPE_KERNEL &&
+ hdr->ih_type == IH_TYPE_KERNEL_NOLOAD)) {
So this is now effectively
(hdr->ih_type != image_type && !(image_type == IH_TYPE_KERNEL &&
hdr->ih_type == IH_TYPE_KERNEL_NOLOAD))
Maybe you don't need two if-s just one with this condition?
I didn't see anything in the style guide that stipulated I couldn't keep
the two ifs. I've left it as is.
+ fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
+ image_type);
+ goto out;
+ }
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+ case IH_TYPE_KERNEL_NOLOAD:
+ if (!loadaddr || *loadaddr == LOAD_UIMAGE_LOADADDR_INVALID) {
+ fprintf(stderr, "this image format (kernel_noload) cannot
be "
+ "loaded on this machine type");
+ goto out;
+ }
+
+ hdr->ih_load = *loadaddr + sizeof(*hdr);
+ hdr->ih_ep += hdr->ih_load;
+ /* fall through */
case IH_TYPE_KERNEL:
address = hdr->ih_load;
if (translate_fn) {
diff --git a/hw/core/uboot_image.h b/hw/core/uboot_image.h
index 34c11a70a6..608022de6e 100644
--- a/hw/core/uboot_image.h
+++ b/hw/core/uboot_image.h
@@ -124,6 +124,7 @@
#define IH_TYPE_SCRIPT 6 /* Script file */
#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image (any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat Device Tree Blob */
+#define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image (noload) */
Do we want to make this an enum like in recent U-Boot? (Not suggesting
we should just asking if you're touching this anyway.)
I guess this is for someone else to answer. I wanted to provide a
minimal diff.
/*
* Compression Types
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 35bfeda7aa..489ab839b7 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -156,7 +156,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu,
hwaddr ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
- hwaddr uentry, loadaddr;
+ hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry,
&lo
Re: [Qemu-devel] [PATCH v4] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
On 06/01/2019 22:56, no-re...@patchew.org wrote: Patchew URL: https://patchew.org/QEMU/d19529f5-841e-ea06-fe7d-86ccfd528...@gmail.com/ Hi, This series seems to have some coding style problems. See output below for more information: The files being touched have lots of coding style problems already. I'm avoiding i) fixing these in the same diff for clarity and ii) in the case of IH_TYPE_KERNEL_NOLOAD, following existing formatting. I'll send v5 with the loadaddr api documentation formatting fix. Nick
[Qemu-devel] [PATCH v4] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately.
The default location for the uboot file is 32MiB above bottom of DRAM.
This matches the recommendation in Documentation/arm/Booting.
Clarify the load_uimage API to state the passing of a load address when an
image doesn't specify one, or when loading a ramdisk is expected.
Adjust callers of load_uimage, etc.
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 19 ---
hw/core/uboot_image.h | 1 +
hw/microblaze/boot.c | 2 +-
hw/nios2/boot.c| 2 +-
hw/ppc/e500.c | 1 +
hw/ppc/ppc440_bamboo.c | 2 +-
hw/ppc/sam460ex.c | 2 +-
include/hw/loader.h| 7 ++-
9 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 94fce12802..c7a67af7a9 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x0200
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET8
@@ -1082,7 +1083,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
}
entry = elf_entry;
if (kernel_size < 0) {
-kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
+uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+kernel_size = load_uimage_as(info->kernel_filename, &entry, &loadaddr,
&is_linux, NULL, NULL, as);
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index fa41842280..c7182dfa64 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -613,13 +613,26 @@ static int load_uboot_image(const char *filename, hwaddr
*ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
-fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
-image_type);
-goto out;
+if (!(image_type == IH_TYPE_KERNEL &&
+hdr->ih_type == IH_TYPE_KERNEL_NOLOAD)) {
+fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
+image_type);
+goto out;
+}
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+case IH_TYPE_KERNEL_NOLOAD:
+if (!loadaddr || *loadaddr == LOAD_UIMAGE_LOADADDR_INVALID) {
+fprintf(stderr, "this image format (kernel_noload) cannot be "
+"loaded on this machine type");
+goto out;
+}
+
+hdr->ih_load = *loadaddr + sizeof(*hdr);
+hdr->ih_ep += hdr->ih_load;
+/* fall through */
case IH_TYPE_KERNEL:
address = hdr->ih_load;
if (translate_fn) {
diff --git a/hw/core/uboot_image.h b/hw/core/uboot_image.h
index 34c11a70a6..608022de6e 100644
--- a/hw/core/uboot_image.h
+++ b/hw/core/uboot_image.h
@@ -124,6 +124,7 @@
#define IH_TYPE_SCRIPT 6 /* Script file */
#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image (any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat Device Tree Blob */
+#define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image (noload) */
/*
* Compression Types
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 35bfeda7aa..489ab839b7 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -156,7 +156,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu, hwaddr
ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
-hwaddr uentry, loadaddr;
+hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, 0,
NULL, NULL);
diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index 4bb5b601d3..ed5cb28e94 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -161,7 +161,7 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
-hwaddr uentry, loadaddr;
+hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, 0,
NULL, NULL);
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index b20fea0dfc..0581e9e3d4 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -995,6 +995,7 @@ void ppce500_init(MachineState *machine)
Re: [Qemu-devel] AVX support for TCG
Right, that makes sense, thanks for the explanations. As someone with very little x86 experience (zero experience from this perspective) it's kind of daunting that I'd have to refactor all this stuff. All these helpers via macros to get around C's 'minimalism' also seem like something I'd have to get accustomed to. I will think about it. Just curious, why is gvec-desc a bitfield instead of a normal struct? Surely it'd be more readable that way. Also this is C, so it's not even a typed bitfield, just a uint32. I'm guessing there's a reason behind this?
Re: [Qemu-devel] AVX support for TCG
Στις Σάβ, 5 Ιαν 2019 στις 12:14 π.μ., ο/η Richard Henderson έγραψε: > No, it's just calling conventions. And it could be worked around, but I think > what we have is convenient enough. > > Especially since the sizes are encoded as (n+1)*8, which also shows the > compiler that the size is positive, so the for loop must iterate at least > once. I know host ABI's can differ like that, but I don't understand why that should matter. Everything (TCG compiler included) is compiled with the same way, right? For the host arch. Or is that a host ABI vs guest ABI thing? Though I don't understand why that would matter either. All this is stuff that runs on the host, right? Oh or does it have to do with JIT'ted tcg helper functions where ABI would matter? No real need to explain, I'm just curious.
Re: [Qemu-devel] AVX support for TCG
Ohh got it, thanks. Στις Σάβ, 5 Ιαν 2019 στις 12:38 π.μ., ο/η Richard Henderson έγραψε: > > On 1/5/19 8:33 AM, Nick Renieris wrote: > > I know host ABI's can differ like that, but I don't understand why > > that should matter. Everything (TCG compiler included) is compiled > > with the same way, right? For the host arch. > > No, not all of the pieces are compiled the same way. TCG generates machine > instructions to perform a call from generated code into compiler generated > code. So TCG needs to know about the host compiler ABI. > > > r~
Re: [Qemu-devel] AVX support for TCG
Στις Παρ, 4 Ιαν 2019 στις 11:51 μ.μ., ο/η Richard Henderson έγραψε: > As an integer it is always passed by value. As a structure some host abis > pass > it by reference, and the TCG compiler doesn't know about that. Ah so they modify it? If so it could surely be worked around with explicit stack copies, right?
Re: [Qemu-devel] [PATCH v3] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
On 03/01/2019 17:27, Peter Maydell wrote: On Thu, 3 Jan 2019 at 16:50, Nick Hudson wrote: On 03/01/2019 16:20, Peter Maydell wrote: On Tue, 11 Dec 2018 at 12:27, Nick Hudson wrote: --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -30,8 +30,9 @@ * Documentation/arm/Booting and Documentation/arm64/booting.txt * They have different preferred image load offsets from system RAM base. */ -#define KERNEL_ARGS_ADDR 0x100 -#define KERNEL_LOAD_ADDR 0x0001 +#define KERNEL_ARGS_ADDR 0x100 +#define KERNEL_NOLOAD_ADDR 0x If I'm reading the code right, this requests that noload images are loaded at offset zero into RAM, yes ? Not quite. They're loaded as if the full noload image (including uboot header) was loaded at offset zero, but as load_uboot_image doesn't load the header then the image body (minus the header) is loaded at offset zero + the size of the u-boot header, i.e. 0x40. That's not a great choice, because we put our little mini bootloader at offset 0, and so the two will clash. Fortuntately, the bootloader fits in the space that the uboot header would have occupied. My commit message has "The bootloader fits in the space that the uboot header would have occupied." to try and described this. I could add more of a description if required? Ah, I missed that. This seems very fragile to me -- 0x40 is only 64 bytes, which is 16 instructions. Currently our boot loader code is less than that, but only by four insns or so, so it's very plausible that some future enhancement to the loader code would take it over the 0x40 boundary, at which point handling of noload images will silently fail. At a minimum we should have an assertion that we stay below 0x40; but I'm not keen on restricting ourselves to 16-instruction bootloaders. I can add an assertion. Kernels often like to be 2MiB aligned to allow for more simple translation tables. This and the fact that the bootloader hasn't changed in 3 years I'd like to kick this can down the road. Will a noload image ever in practice also be a Linux kernel (ie hdr->ih_os == IH_OS_LINUX) ? NetBSD/FreeBSD/OpenBSD (ab)use IH_OS_LINUX. They and Linux If not, then we don't lose anything by not allowing those to be loaded. (And if anybody does in future complain that their image doesn't work, we could make it supported by allowing the mini-bootloader to be placed elsewhere in RAM.) thanks -- PMM thanks, Nick
Re: [Qemu-devel] [PATCH v3] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
On 03/01/2019 16:20, Peter Maydell wrote:
> On Tue, 11 Dec 2018 at 12:27, Nick Hudson wrote:
>>
>>
>> noload kernels are loaded with the u-boot image header and as a result
>> the header size needs adding to the entry point. Fake up a hdr so the
>> kernel image is loaded at the right address and the entry point is
>> adjusted appropriately.
>>
>> The bootloader fits in the space that the uboot header would have occupied.
>>
>> Clarify the load_uimage API to state the passing of a load address when an
>> image doesn't specify one, or when loading a ramdisk is expected.
>>
>> Adjust callers of load_uimage, etc.
>>
>> Signed-off-by: Nick Hudson
>>
>> ---
>>hw/arm/boot.c | 8 +---
>>hw/core/loader.c | 19 ---
>>hw/core/uboot_image.h | 1 +
>>hw/microblaze/boot.c | 2 +-
>>hw/nios2/boot.c| 2 +-
>>hw/ppc/e500.c | 1 +
>>hw/ppc/ppc440_bamboo.c | 2 +-
>>hw/ppc/sam460ex.c | 2 +-
>>include/hw/loader.h| 7 ++-
>>9 files changed, 33 insertions(+), 11 deletions(-)
>
> Hi; I tried applying your patch, but unfortunately it looks like
> your email client has mangled it to the point that it can't
> be applied:
> * it is format=flowed, so long lines have been wrapped
> * some whitespace has been turned into UTF-8 non-breaking space characters
> I tried to manually fix these up, but didn't succeed.
>
> It looks like you're using Thunderbird -- the kernel docs
> have some config setting suggestions that might help:
> https://www.kernel.org/doc/html/v4.11/process/email-clients.html#thunderbird-gui
> or you could try switching to 'git send-email'.
>
> I've reviewed the patch for content and it's mostly good:
> I just have a couple more questions below.
>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 586baa9b64..450267566a 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -30,8 +30,9 @@
>> * Documentation/arm/Booting and Documentation/arm64/booting.txt
>> * They have different preferred image load offsets from system RAM base.
>> */
>> -#define KERNEL_ARGS_ADDR 0x100
>> -#define KERNEL_LOAD_ADDR 0x0001
>> +#define KERNEL_ARGS_ADDR 0x100
>> +#define KERNEL_NOLOAD_ADDR 0x
>
> If I'm reading the code right, this requests that noload images
> are loaded at offset zero into RAM, yes ?
Not quite.
They're loaded as if the full noload image (including uboot header)
was loaded at offset zero, but as load_uboot_image doesn't load the
header then the image body (minus the header) is loaded at offset
zero + the size of the u-boot header, i.e. 0x40.
> That's not a great
> choice, because we put our little mini bootloader at offset 0,
> and so the two will clash.
Fortuntately, the bootloader fits in the space that the uboot header would
have occupied.
My commit message has
"The bootloader fits in the space that the uboot header would have occupied."
to try and described this. I could add more of a description if required?
> Can we put them somewhere else
> (above BOOTLOADER_MAX_SIZE, probably at a 64K page boundary) ?
I don't think this is necessary.
> If they must go at offset 0 then we'd need to refuse to load
> noload images if they set is_linux to true. (We only load the
> mini bootloader if is_linux is true, so that's the only case where
> there's a clash.)
Nor this.
>
>> +#define KERNEL_LOAD_ADDR 0x0001
>>#define KERNEL64_LOAD_ADDR 0x0008
>>
>>#define ARM64_TEXT_OFFSET_OFFSET8
>> @@ -1078,7 +1079,8 @@ void arm_load_kernel(ARMCPU *cpu, struct
>> arm_boot_info *info)
>>}
>>entry = elf_entry;
>>if (kernel_size < 0) {
>> -kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
>> +uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
>> +kernel_size = load_uimage_as(info->kernel_filename, &entry,
>> &loadaddr,
>> &is_linux, NULL, NULL, as);
>>}
>>if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
>> diff --git a/hw/core/loader.c b/hw/core/loader.c
>> index aa0b3fc867..7362197162 100644
>> --- a/hw/core/loader.c
>> +++ b/hw/core/loader.c
>> @@ -638,13 +638,26 @@ static int load_uboot_image(const char *filename,
>> hwaddr *ep, hwaddr *loadaddr,
>>goto out;
>>
>>if (hdr->ih_typ
Re: [Qemu-devel] AVX support for TCG
Στις Σάβ, 29 Δεκ 2018 στις 10:24 μ.μ., ο/η Richard Henderson έγραψε: > I did have a beginner in mind when guessing 4 months. Don't take that as a > fully speced out answer, but it may well be that full avx2 support cannot be > done within the 3 months of gsoc. I would certainly expect avx512 to take > even > longer. Right, that's ok. Bit of context, the reason I'm interested in this feature (other than GSoC potential and general interest) is Orbital, a Sony PlayStation 4 emulator that implements a QEMU fork. Normally we use virtualization as anything else would be too slow, but TCG would help with debugging and such. The PS4's APU doesn't support AVX2 or AVX-512 so I'd be fine if I didn't have enough time to implement them. > The tcg-op-gvec.h infrastructure allows for the different modes that avx+mmx > allows: > > (1) 64-bit operations, > (2) 128-bit operations, modifying only the low 128 bits, > (3) 128-bit operations, zeroing bits beyond the first 128, > (4) N*128-bit operations, zeroing bits beyond the first N*128. I assume you mean 256-bit ops on (2) and (3), and N*256 on (4)? Low 128 bits of a 128-bit number is just the number. > so we should not need a great proliferation of helper functions, merely a > re-organization of what we have now. So, I would need to implement every SSE instruction that isn't SSE_SPECIAL at the moment, using tcg-op-gvec.h? Or more instructions than that? Assuming I do this for SSE and AVX, I would not need to touch anything else like the TCG back-end, as every gvec/vec op is already implemented for i386, correct? PS: I'm 'vel0city' on IRC, if that'd be more convenient.
Re: [Qemu-devel] AVX support for TCG
Στις Παρ, 28 Δεκ 2018 στις 4:39 μ.μ., ο/η Peter Maydell έγραψε: > If your editor can't show multiple views onto one file with > the same simplicity and UI as it has for multiple different > files then I would suggest getting a better editor :-) Apparently I just didn't know how to use my editor :) In my defense, I've never had to do this before. > Unless you want to restrict all your files to 100 lines or > shorter then you need to be able to see multiple parts of them > at once -- one 10,000 line file is no worse than 4 2500 line > files for this. As you mention below, logical separation is key here, I definitely didn't mean that there should be arbitrary LoC limits for each file. > There are definitely improvements that could be made to > the x86 code, and where splitting of files makes conceptual > sense it's certainly worthwhile. The trick is finding the > right logical splitting points. Agreed, though it's not something I can personally help on, unless I spend quite a long time getting acquainted with _this_ code first. I'd like to get some answers to my previous question about estimates of the total amount of work required before considering diving into it.
Re: [Qemu-devel] AVX support for TCG
Right, thanks, that file looks better, though I still think splitting to multiple files would absolutely be of value, if only for the practicality of being able to have several parts of it open at the same in a code editor (instead of having to jump back and forth or find workarounds to open the same file multiple times). Of course there are more important, code structure related reasons beyond that, but let's not digress. I'm guessing current devs are so used to this that they wouldn't accept splitting-up changes anyway. Στις Παρ, 28 Δεκ 2018 στις 4:13 μ.μ., ο/η Peter Maydell έγραψε: > > On Fri, 28 Dec 2018 at 13:45, Nick Renieris wrote: > > Also, I hope you meant four months for me, not for you - I'm > > completely new to the QEMU codebase. I expect it will take me weeks > > just to understand x86's 'translate.c' (who thought it'd be a good > > idea to put all this stuff in _one_ file?). > > x86 suffers from being one of the first and oldest frontends, > and on top of that from not having had a great deal of active > attention. So it has a lot of remnants from older styles of > implementation, as well as the kind of "one big function in > one huge file" that organic growth of a codebase tends to give you. > It doesn't make that much difference whether you have one file > or several, though -- target/arm/translate-a64.c is pretty > well structured and easy (IMHO) to comprehend, but it's > 5000 lines longer than target/i386/translate.c. The comprehensibility > improvements come from better breakdown into separate functions > and much clearer commenting of the extent of the decode at any > particular point (plus not having any legacy baggage to deal with). > > thanks > -- PMM
Re: [Qemu-devel] AVX support for TCG
>> Do you think this could work as a GSoC project? I'm potentially >> interested in working on it this summer. >Could be. My first guess is something like 4 months work for this. Four months full-time? If so I would say it's not viable for a GSoC project (it's 3 months), I've done the 12-hours-a-day-crunch thing for a week or so in GSoC 2017 and it was _not_ fun. Also, I hope you meant four months for me, not for you - I'm completely new to the QEMU codebase. I expect it will take me weeks just to understand x86's 'translate.c' (who thought it'd be a good idea to put all this stuff in _one_ file?). Another question, are there existing discussions about this refactoring effort or specifically AVX? I asked a similar question on IRC a few days ago and got no answers. Στις Τετ, 26 Δεκ 2018 στις 4:12 π.μ., ο/η Richard Henderson έγραψε: > > On 12/26/18 12:28 PM, Nick Renieris wrote: > > Hi Richard, > > > > I did know about https://github.com/andikleen/qemu-avx but didn't > > mention it as it seems abandoned and quite old (also it doesn't use > > `TCGv_vec`). > > Yep. Mine pre-dates tcg-op-gvec.h as well. > > > Do you think this could work as a GSoC project? I'm potentially > > interested in working on it this summer. > > Could be. My first guess is something like 4 months work for this. > > > r~
Re: [Qemu-devel] AVX support for TCG
Hi Richard, I did know about https://github.com/andikleen/qemu-avx but didn't mention it as it seems abandoned and quite old (also it doesn't use `TCGv_vec`). Do you think this could work as a GSoC project? I'm potentially interested in working on it this summer. Thanks, Nick R. Στις Τετ, 26 Δεκ 2018 στις 3:07 π.μ., ο/η Richard Henderson έγραψε: > > On 12/26/18 10:43 AM, Nick Renieris wrote: > > Hello, > > > > What's the current status on AVX support for TCG? Is there anyone working > > on it? > > Is there interest for it? > > A couple of people, including myself, have started on it, but no results > posted > so far. It's a large amount of work to clean up the exiting sse support > before > avx could be added. > > > r~
[Qemu-devel] AVX support for TCG
Hello, What's the current status on AVX support for TCG? Is there anyone working on it? Is there interest for it? Effort was put into making 'TCGv_vec', as elaborated in this talk: "[2017] Vectoring in on QEMU's TCG Engine by Alex Bennée" (https://www.youtube.com/watch?v=IYHTwnde0g8) ,yet there doesn't seem to be a front-end (except for VEX prefix parsing) nor a backend for AVX, a pretty common x86 extension. I'd appreciate any info on this, Regards, Nick Renieris.
Re: [Qemu-devel] [PATCH v3] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
ping
On 11/12/2018 12:27, Nick Hudson wrote:
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately.
The bootloader fits in the space that the uboot header would have
occupied.
Clarify the load_uimage API to state the passing of a load address
when an
image doesn't specify one, or when loading a ramdisk is expected.
Adjust callers of load_uimage, etc.
Signed-off-by: Nick Hudson
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 19 ---
hw/core/uboot_image.h | 1 +
hw/microblaze/boot.c | 2 +-
hw/nios2/boot.c | 2 +-
hw/ppc/e500.c | 1 +
hw/ppc/ppc440_bamboo.c | 2 +-
hw/ppc/sam460ex.c | 2 +-
include/hw/loader.h | 7 ++-
9 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 586baa9b64..450267566a 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM
base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET 8
@@ -1078,7 +1079,8 @@ void arm_load_kernel(ARMCPU *cpu, struct
arm_boot_info *info)
}
entry = elf_entry;
if (kernel_size < 0) {
- kernel_size = load_uimage_as(info->kernel_filename, &entry,
NULL,
+ uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+ kernel_size = load_uimage_as(info->kernel_filename, &entry,
&loadaddr,
&is_linux, NULL, NULL, as);
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size <
0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc867..7362197162 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -638,13 +638,26 @@ static int load_uboot_image(const char
*filename, hwaddr *ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
- fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
- image_type);
- goto out;
+ if (image_type != IH_TYPE_KERNEL &&
+ hdr->ih_type != IH_TYPE_KERNEL_NOLOAD) {
+ fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
+ image_type);
+ goto out;
+ }
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+ case IH_TYPE_KERNEL_NOLOAD:
+ if (!loadaddr || *loadaddr == LOAD_UIMAGE_LOADADDR_INVALID) {
+ fprintf(stderr, "this image format (kernel_noload) cannot
be "
+ "loaded on this machine type");
+ goto out;
+ }
+
+ hdr->ih_load = *loadaddr + sizeof(*hdr);
+ hdr->ih_ep += hdr->ih_load;
+ /* fall through */
case IH_TYPE_KERNEL:
address = hdr->ih_load;
if (translate_fn) {
diff --git a/hw/core/uboot_image.h b/hw/core/uboot_image.h
index 34c11a70a6..608022de6e 100644
--- a/hw/core/uboot_image.h
+++ b/hw/core/uboot_image.h
@@ -124,6 +124,7 @@
#define IH_TYPE_SCRIPT 6 /* Script file */
#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image (any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat Device Tree Blob */
+#define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image (noload) */
/*
* Compression Types
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 35bfeda7aa..489ab839b7 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -156,7 +156,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu,
hwaddr ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
- hwaddr uentry, loadaddr;
+ hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry,
&loadaddr, 0,
NULL, NULL);
diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index 4bb5b601d3..ed5cb28e94 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -161,7 +161,7 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr
ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
- hwaddr uentry, loadaddr;
+ hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry,
&loadaddr, 0,
NULL, NULL);
diff --git
[Qemu-devel] [PATCH v3] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately.
The bootloader fits in the space that the uboot header would have occupied.
Clarify the load_uimage API to state the passing of a load address when an
image doesn't specify one, or when loading a ramdisk is expected.
Adjust callers of load_uimage, etc.
Signed-off-by: Nick Hudson
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 19 ---
hw/core/uboot_image.h | 1 +
hw/microblaze/boot.c | 2 +-
hw/nios2/boot.c | 2 +-
hw/ppc/e500.c | 1 +
hw/ppc/ppc440_bamboo.c | 2 +-
hw/ppc/sam460ex.c | 2 +-
include/hw/loader.h | 7 ++-
9 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 586baa9b64..450267566a 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET 8
@@ -1078,7 +1079,8 @@ void arm_load_kernel(ARMCPU *cpu, struct
arm_boot_info *info)
}
entry = elf_entry;
if (kernel_size < 0) {
- kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
+ uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+ kernel_size = load_uimage_as(info->kernel_filename, &entry,
&loadaddr,
&is_linux, NULL, NULL, as);
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc867..7362197162 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -638,13 +638,26 @@ static int load_uboot_image(const char *filename,
hwaddr *ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
- fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
- image_type);
- goto out;
+ if (image_type != IH_TYPE_KERNEL &&
+ hdr->ih_type != IH_TYPE_KERNEL_NOLOAD) {
+ fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
+ image_type);
+ goto out;
+ }
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+ case IH_TYPE_KERNEL_NOLOAD:
+ if (!loadaddr || *loadaddr == LOAD_UIMAGE_LOADADDR_INVALID) {
+ fprintf(stderr, "this image format (kernel_noload) cannot be "
+ "loaded on this machine type");
+ goto out;
+ }
+
+ hdr->ih_load = *loadaddr + sizeof(*hdr);
+ hdr->ih_ep += hdr->ih_load;
+ /* fall through */
case IH_TYPE_KERNEL:
address = hdr->ih_load;
if (translate_fn) {
diff --git a/hw/core/uboot_image.h b/hw/core/uboot_image.h
index 34c11a70a6..608022de6e 100644
--- a/hw/core/uboot_image.h
+++ b/hw/core/uboot_image.h
@@ -124,6 +124,7 @@
#define IH_TYPE_SCRIPT 6 /* Script file */
#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image (any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat Device Tree Blob */
+#define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image (noload) */
/*
* Compression Types
diff --git a/hw/microblaze/boot.c b/hw/microblaze/boot.c
index 35bfeda7aa..489ab839b7 100644
--- a/hw/microblaze/boot.c
+++ b/hw/microblaze/boot.c
@@ -156,7 +156,7 @@ void microblaze_load_kernel(MicroBlazeCPU *cpu,
hwaddr ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
- hwaddr uentry, loadaddr;
+ hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry,
&loadaddr, 0,
NULL, NULL);
diff --git a/hw/nios2/boot.c b/hw/nios2/boot.c
index 4bb5b601d3..ed5cb28e94 100644
--- a/hw/nios2/boot.c
+++ b/hw/nios2/boot.c
@@ -161,7 +161,7 @@ void nios2_load_kernel(Nios2CPU *cpu, hwaddr ddr_base,
/* If it wasn't an ELF image, try an u-boot image. */
if (kernel_size < 0) {
- hwaddr uentry, loadaddr;
+ hwaddr uentry, loadaddr = LOAD_UIMAGE_LOADADDR_INVALID;
kernel_size = load_uimage(kernel_filename, &uentry,
&loadaddr, 0,
NULL, NULL);
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index e6747fce28..e275178951
Re: [Qemu-devel] [PATCH v2] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
On 30/11/2018 17:18, Peter Maydell wrote:
On Thu, 29 Nov 2018 at 20:22, Nick Hudson wrote:
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately.
The bootloader fits in the space that the uboot header would have occupied.
Update the load_uimage API to allow passing of load address when an image
doesn't specify one.
Signed-off-by: Nick Hudson
---
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 67a0af84ac..10ff0ff76d 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -163,7 +163,8 @@ int load_aout(const char *filename, hwaddr addr, int max_sz,
/** load_uimage_as:
* @filename: Path of uimage file
* @ep: Populated with program entry point. Ignored if NULL.
- * @loadaddr: Populated with the load address. Ignored if NULL.
+ * @loadaddr: load address if none specified in the image. Populated with the
+ *load address. Ignored if NULL.
This API change will break some existing users. For instance
hw/microblaze/boot.c does this:
hwaddr uentry, loadaddr;
kernel_size = load_uimage(kernel_filename, &uentry, &loadaddr, 0,
NULL, NULL);
I did a bit more digging and the same (ab)use of loadaddr to pass
a value is used by load_ramdisk{,_as}. Hopefully, I can do the same
for kernel_noload?
which is a legitimate use of the current API, but your changes will
mean that load_uboot_image() will be using an uninitialized
variable. Getting every call site that cares about the returned
result from loadaddr to also provide a valid default load
address as input is going to be really painful, so I think we
need to figure out an API for this function that doesn't
make the input (default load address) argument the same
parameter as the output (where we actually loaded the image)
argument.
I have version 3 ready with fixing these callers.
We should also make the load_uboot_image() function print an
error message if we don't load the file because it needs a
default load address and the caller didn't pass one
("this image format cannot be loaded on this machine type",
or similar).
Fixed in version 3.
PS: our coding style wants {} on all if()s -- scripts/checkpatch.pl
can help catch this kind of nit.
There are lots of errors from this without my changes :(
thanks
-- PMM
Thanks,
Nick
[Qemu-devel] [PATCH v2] Support u-boot noload images for arm as used by, NetBSD/evbarm GENERIC kernel.
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately.
The bootloader fits in the space that the uboot header would have occupied.
Update the load_uimage API to allow passing of load address when an image
doesn't specify one.
Signed-off-by: Nick Hudson
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 15 ---
hw/core/uboot_image.h | 1 +
include/hw/loader.h | 3 ++-
4 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 586baa9b64..450267566a 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET8
@@ -1078,7 +1079,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
}
entry = elf_entry;
if (kernel_size < 0) {
-kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
+uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+kernel_size = load_uimage_as(info->kernel_filename, &entry, &loadaddr,
&is_linux, NULL, NULL, as);
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc867..5537e88817 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -638,13 +638,22 @@ static int load_uboot_image(const char *filename, hwaddr
*ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
-fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
-image_type);
-goto out;
+if (image_type != IH_TYPE_KERNEL && hdr->ih_type !=
IH_TYPE_KERNEL_NOLOAD) {
+fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
+image_type);
+goto out;
+}
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+case IH_TYPE_KERNEL_NOLOAD:
+ if (loadaddr == NULL)
+goto out;
+
+hdr->ih_load = *loadaddr + sizeof(*hdr);
+hdr->ih_ep += hdr->ih_load;
+/* fall through */
case IH_TYPE_KERNEL:
address = hdr->ih_load;
if (translate_fn) {
diff --git a/hw/core/uboot_image.h b/hw/core/uboot_image.h
index 34c11a70a6..608022de6e 100644
--- a/hw/core/uboot_image.h
+++ b/hw/core/uboot_image.h
@@ -124,6 +124,7 @@
#define IH_TYPE_SCRIPT 6 /* Script file */
#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image (any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat Device Tree Blob */
+#define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image (noload) */
/*
* Compression Types
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 67a0af84ac..10ff0ff76d 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -163,7 +163,8 @@ int load_aout(const char *filename, hwaddr addr, int max_sz,
/** load_uimage_as:
* @filename: Path of uimage file
* @ep: Populated with program entry point. Ignored if NULL.
- * @loadaddr: Populated with the load address. Ignored if NULL.
+ * @loadaddr: load address if none specified in the image. Populated with the
+ *load address. Ignored if NULL.
* @is_linux: Is set to true if the image loaded is Linux. Ignored if NULL.
* @translate_fn: optional function to translate load addresses
* @translate_opaque: opaque data passed to @translate_fn
--
2.17.1
Re: [Qemu-devel] [PATCH] Support u-boot noload images for arm as used by NetBSD/evbarm GENERIC kernel.
On 16/11/2018 14:34, Peter Maydell wrote:
On 7 November 2018 at 13:19, Nick Hudson wrote:
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately
Signed-off-by: Nick Hudson
Hi; thanks for this patch.
Thanks for reviewing.
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 12 +---
hw/core/uboot_image.h | 1 +
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 586baa9b64..450267566a 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET8
@@ -1078,7 +1079,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
}
entry = elf_entry;
if (kernel_size < 0) {
-kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
+uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+kernel_size = load_uimage_as(info->kernel_filename, &entry,
&loadaddr,
&is_linux, NULL, NULL, as);
I don't really understand this change. The API for load_uimage_as()
says that the loadaddr argument is an output, not an input, and that
it is ignored if it is NULL. If we're changing the API here then
we need to (a) document that change and (b) check all the callers
can handle it. It would be better if we can avoid having to do that.
I chose to pass where the noload kernel should be loaded via loadaddr as
arm_load_kernel knows where RAM is. Perhaps there's a better way?
BTW, I should have mentioned that I pass the same address that is used
for the bootloader. This address is adjusted in load_uboot_image by the
size of the uboot header. As the bootloader is smaller than the uboot
header it all works. uboot loads the whole image including header and
adjusts the entry point by the header size.
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc867..952562c2da 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -638,13 +638,19 @@ static int load_uboot_image(const char *filename,
hwaddr *ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
-fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
-image_type);
-goto out;
+if (image_type != IH_TYPE_KERNEL && hdr->ih_type !=
IH_TYPE_KERNEL_NOLOAD) {
+fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
+image_type);
+goto out;
+}
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+case IH_TYPE_KERNEL_NOLOAD:
+hdr->ih_load = *loadaddr + sizeof(*hdr);
This will segfault if the user passed in NULL for loadaddr,
as they are allowed to do by the API.
I can update the header with this API change and update callers if
that's ok?
+hdr->ih_ep += hdr->ih_load;
+
Cases which fall through need a specific
/* fall through */
comment. This tells human readers and static analysis tools that
it was intentional.
Sure, I did wonder if this was expected.
Thanks,
Nick
Re: [Qemu-devel] [PATCH] Support u-boot noload images for arm as used by NetBSD/evbarm GENERIC kernel.
ping
On 07/11/2018 13:19, Nick Hudson wrote:
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately
Signed-off-by: Nick Hudson
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 12 +---
hw/core/uboot_image.h | 1 +
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 586baa9b64..450267566a 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM
base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET 8
@@ -1078,7 +1079,8 @@ void arm_load_kernel(ARMCPU *cpu, struct
arm_boot_info *info)
}
entry = elf_entry;
if (kernel_size < 0) {
- kernel_size = load_uimage_as(info->kernel_filename, &entry,
NULL,
+ uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+ kernel_size = load_uimage_as(info->kernel_filename, &entry,
&loadaddr,
&is_linux, NULL, NULL, as);
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size <
0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc867..952562c2da 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -638,13 +638,19 @@ static int load_uboot_image(const char
*filename, hwaddr *ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
- fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
- image_type);
- goto out;
+ if (image_type != IH_TYPE_KERNEL && hdr->ih_type !=
IH_TYPE_KERNEL_NOLOAD) {
+ fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
+ image_type);
+ goto out;
+ }
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+ case IH_TYPE_KERNEL_NOLOAD:
+ hdr->ih_load = *loadaddr + sizeof(*hdr);
+ hdr->ih_ep += hdr->ih_load;
+
case IH_TYPE_KERNEL:
address = hdr->ih_load;
if (translate_fn) {
diff --git a/hw/core/uboot_image.h b/hw/core/uboot_image.h
index 34c11a70a6..608022de6e 100644
--- a/hw/core/uboot_image.h
+++ b/hw/core/uboot_image.h
@@ -124,6 +124,7 @@
#define IH_TYPE_SCRIPT 6 /* Script file */
#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image (any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat Device Tree Blob */
+#define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image (noload) */
/*
* Compression Types
[Qemu-devel] [PATCH] Support u-boot noload images for arm as used by NetBSD/evbarm GENERIC kernel.
noload kernels are loaded with the u-boot image header and as a result
the header size needs adding to the entry point. Fake up a hdr so the
kernel image is loaded at the right address and the entry point is
adjusted appropriately
Signed-off-by: Nick Hudson
---
hw/arm/boot.c | 8 +---
hw/core/loader.c | 12 +---
hw/core/uboot_image.h | 1 +
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 586baa9b64..450267566a 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -30,8 +30,9 @@
* Documentation/arm/Booting and Documentation/arm64/booting.txt
* They have different preferred image load offsets from system RAM base.
*/
-#define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_ARGS_ADDR 0x100
+#define KERNEL_NOLOAD_ADDR 0x
+#define KERNEL_LOAD_ADDR 0x0001
#define KERNEL64_LOAD_ADDR 0x0008
#define ARM64_TEXT_OFFSET_OFFSET 8
@@ -1078,7 +1079,8 @@ void arm_load_kernel(ARMCPU *cpu, struct
arm_boot_info *info)
}
entry = elf_entry;
if (kernel_size < 0) {
- kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
+ uint64_t loadaddr = info->loader_start + KERNEL_NOLOAD_ADDR;
+ kernel_size = load_uimage_as(info->kernel_filename, &entry,
&loadaddr,
&is_linux, NULL, NULL, as);
}
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
diff --git a/hw/core/loader.c b/hw/core/loader.c
index aa0b3fc867..952562c2da 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -638,13 +638,19 @@ static int load_uboot_image(const char *filename,
hwaddr *ep, hwaddr *loadaddr,
goto out;
if (hdr->ih_type != image_type) {
- fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
- image_type);
- goto out;
+ if (image_type != IH_TYPE_KERNEL && hdr->ih_type !=
IH_TYPE_KERNEL_NOLOAD) {
+ fprintf(stderr, "Wrong image type %d, expected %d\n",
hdr->ih_type,
+ image_type);
+ goto out;
+ }
}
/* TODO: Implement other image types. */
switch (hdr->ih_type) {
+ case IH_TYPE_KERNEL_NOLOAD:
+ hdr->ih_load = *loadaddr + sizeof(*hdr);
+ hdr->ih_ep += hdr->ih_load;
+
case IH_TYPE_KERNEL:
address = hdr->ih_load;
if (translate_fn) {
diff --git a/hw/core/uboot_image.h b/hw/core/uboot_image.h
index 34c11a70a6..608022de6e 100644
--- a/hw/core/uboot_image.h
+++ b/hw/core/uboot_image.h
@@ -124,6 +124,7 @@
#define IH_TYPE_SCRIPT 6 /* Script file */
#define IH_TYPE_FILESYSTEM 7 /* Filesystem Image (any type) */
#define IH_TYPE_FLATDT 8 /* Binary Flat Device Tree Blob */
+#define IH_TYPE_KERNEL_NOLOAD 14 /* OS Kernel Image (noload) */
/*
* Compression Types
--
2.17.1
[Qemu-devel] outdated wiki link
Hello, Cruising by the wki, thought that ARAnyM was a cool project. Found it here: https://wiki.qemu.org/Links#Other_emulators However, the link was out of date and now points to some domain squatted site. The updated link is: https://aranym.github.io/ Cheers! -- Nick
[Qemu-devel] [Bug 1696353] Re: golang binaries fail to start under linux-user
The go team have applied the above patch and I can confirm that it is now working properly using go-tip. This means it will be fixed in go 1.10. So if you recompile your go binary with go-tip or go 1.10 when it is released, it will run fine under qemu-system-arm. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1696353 Title: golang binaries fail to start under linux-user Status in QEMU: New Bug description: With current master golang binaries fail when run under linux-user, for example: [will@localhost qemu]$ ./arm-linux-user/qemu-arm glide runtime: failed to create new OS thread (have 2 already; errno=22) fatal error: newosproc runtime stack: runtime.throw(0x45f879, 0x9) /usr/lib/golang/src/runtime/panic.go:566 +0x78 runtime.newosproc(0x1092c000, 0x1093bfe0) /usr/lib/golang/src/runtime/os_linux.go:160 +0x1b0 runtime.newm(0x4ae1e8, 0x0) /usr/lib/golang/src/runtime/proc.go:1572 +0x12c runtime.main.func1() /usr/lib/golang/src/runtime/proc.go:126 +0x24 runtime.systemstack(0x5ef900) /usr/lib/golang/src/runtime/asm_arm.s:247 +0x80 runtime.mstart() /usr/lib/golang/src/runtime/proc.go:1079 goroutine 1 [running]: runtime.systemstack_switch() /usr/lib/golang/src/runtime/asm_arm.s:192 +0x4 fp=0x109287ac sp=0x109287a8 runtime.main() /usr/lib/golang/src/runtime/proc.go:127 +0x5c fp=0x109287d4 sp=0x109287ac runtime.goexit() /usr/lib/golang/src/runtime/asm_arm.s:998 +0x4 fp=0x109287d4 sp=0x109287d4 The reason for this is that the golang runtime does not pass the CLONE_SYSVMEM flag to clone so the clone flags checks fail: https://github.com/golang/go/blob/master/src/runtime/os_linux.go#L155 The attached patch allows golang binaries to start under linux-user. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1696353/+subscriptions
[Qemu-devel] [Bug 1696353] Re: golang binaries fail to start under linux-user
You can also apply this patch to go - I don't have an opinion on the correct course of action though! diff --git a/src/runtime/os_linux.go b/src/runtime/os_linux.go index a6efc0e3d1..64218e3f7e 100644 --- a/src/runtime/os_linux.go +++ b/src/runtime/os_linux.go @@ -132,7 +132,8 @@ const ( _CLONE_FS | /* share cwd, etc */ _CLONE_FILES | /* share fd table */ _CLONE_SIGHAND | /* share sig handler table */ - _CLONE_THREAD /* revisit - okay for now */ + _CLONE_THREAD | /* revisit - okay for now */ + _CLONE_SYSVSEM ) //go:noescape ** Bug watch added: github.com/golang/go/issues #20763 https://github.com/golang/go/issues/20763 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1696353 Title: golang binaries fail to start under linux-user Status in QEMU: New Bug description: With current master golang binaries fail when run under linux-user, for example: [will@localhost qemu]$ ./arm-linux-user/qemu-arm glide runtime: failed to create new OS thread (have 2 already; errno=22) fatal error: newosproc runtime stack: runtime.throw(0x45f879, 0x9) /usr/lib/golang/src/runtime/panic.go:566 +0x78 runtime.newosproc(0x1092c000, 0x1093bfe0) /usr/lib/golang/src/runtime/os_linux.go:160 +0x1b0 runtime.newm(0x4ae1e8, 0x0) /usr/lib/golang/src/runtime/proc.go:1572 +0x12c runtime.main.func1() /usr/lib/golang/src/runtime/proc.go:126 +0x24 runtime.systemstack(0x5ef900) /usr/lib/golang/src/runtime/asm_arm.s:247 +0x80 runtime.mstart() /usr/lib/golang/src/runtime/proc.go:1079 goroutine 1 [running]: runtime.systemstack_switch() /usr/lib/golang/src/runtime/asm_arm.s:192 +0x4 fp=0x109287ac sp=0x109287a8 runtime.main() /usr/lib/golang/src/runtime/proc.go:127 +0x5c fp=0x109287d4 sp=0x109287ac runtime.goexit() /usr/lib/golang/src/runtime/asm_arm.s:998 +0x4 fp=0x109287d4 sp=0x109287d4 The reason for this is that the golang runtime does not pass the CLONE_SYSVMEM flag to clone so the clone flags checks fail: https://github.com/golang/go/blob/master/src/runtime/os_linux.go#L155 The attached patch allows golang binaries to start under linux-user. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1696353/+subscriptions
[Qemu-devel] [Bug 1696353] Re: golang binaries fail to start under linux-user
Note that there is a go bug about this issue too: https://github.com/golang/go/issues/20763 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1696353 Title: golang binaries fail to start under linux-user Status in QEMU: New Bug description: With current master golang binaries fail when run under linux-user, for example: [will@localhost qemu]$ ./arm-linux-user/qemu-arm glide runtime: failed to create new OS thread (have 2 already; errno=22) fatal error: newosproc runtime stack: runtime.throw(0x45f879, 0x9) /usr/lib/golang/src/runtime/panic.go:566 +0x78 runtime.newosproc(0x1092c000, 0x1093bfe0) /usr/lib/golang/src/runtime/os_linux.go:160 +0x1b0 runtime.newm(0x4ae1e8, 0x0) /usr/lib/golang/src/runtime/proc.go:1572 +0x12c runtime.main.func1() /usr/lib/golang/src/runtime/proc.go:126 +0x24 runtime.systemstack(0x5ef900) /usr/lib/golang/src/runtime/asm_arm.s:247 +0x80 runtime.mstart() /usr/lib/golang/src/runtime/proc.go:1079 goroutine 1 [running]: runtime.systemstack_switch() /usr/lib/golang/src/runtime/asm_arm.s:192 +0x4 fp=0x109287ac sp=0x109287a8 runtime.main() /usr/lib/golang/src/runtime/proc.go:127 +0x5c fp=0x109287d4 sp=0x109287ac runtime.goexit() /usr/lib/golang/src/runtime/asm_arm.s:998 +0x4 fp=0x109287d4 sp=0x109287d4 The reason for this is that the golang runtime does not pass the CLONE_SYSVMEM flag to clone so the clone flags checks fail: https://github.com/golang/go/blob/master/src/runtime/os_linux.go#L155 The attached patch allows golang binaries to start under linux-user. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1696353/+subscriptions
[Qemu-devel] [Bug 1701449] [NEW] high memory usage when using rbd with client caching
Public bug reported: Hi, we are experiencing a quite high memory usage of a single qemu (used with KVM) process when using RBD with client caching as a disk backend. We are testing with 3GB memory qemu virtual machines and 128MB RBD client cache. When running 'fio' in the virtual machine you can see that after some time the machine uses a lot more memory (RSS) on the hypervisor than she should. We have seen values (in real production machines, no artificially fio tests) of 250% memory overhead. I reproduced this with qemu version 2.9 as well. Here the contents of our ceph.conf on the hypervisor: """ [client] rbd cache writethrough until flush = False rbd cache max dirty = 100663296 rbd cache size = 134217728 rbd cache target dirty = 50331648 """ How to reproduce: * create a virtual machine with a RBD backed disk (100GB or so) * install a linux distribution on it (we are using Ubuntu) * install fio (apt-get install fio) * run fio multiple times with (e.g.) the following test file: """ # This job file tries to mimic the Intel IOMeter File Server Access Pattern [global] description=Emulation of Intel IOmeter File Server Access Pattern randrepeat=0 filename=/root/test.dat # IOMeter defines the server loads as the following: # iodepth=1 Linear # iodepth=4 Very Light # iodepth=8 Light # iodepth=64Moderate # iodepth=256 Heavy iodepth=8 size=80g direct=0 ioengine=libaio [iometer] stonewall bs=4M rw=randrw [iometer_just_write] stonewall bs=4M rw=write [iometer_just_read] stonewall bs=4M rw=read """ You can measure the virtual machine RSS usage on the hypervisor with: virsh dommemstat | grep rss or if you are not using libvirt: grep RSS /proc//status When switching off the RBD client cache, all is ok again, as the process does not use so much memory anymore. There is already a ticket on the ceph bug tracker for this ([1]). However I can reproduce that memory behaviour only when using qemu (maybe it is using librbd in a special way?). Running directly 'fio' with the rbd engine does not result in that high memory usage. [1] http://tracker.ceph.com/issues/20054 ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1701449 Title: high memory usage when using rbd with client caching Status in QEMU: New Bug description: Hi, we are experiencing a quite high memory usage of a single qemu (used with KVM) process when using RBD with client caching as a disk backend. We are testing with 3GB memory qemu virtual machines and 128MB RBD client cache. When running 'fio' in the virtual machine you can see that after some time the machine uses a lot more memory (RSS) on the hypervisor than she should. We have seen values (in real production machines, no artificially fio tests) of 250% memory overhead. I reproduced this with qemu version 2.9 as well. Here the contents of our ceph.conf on the hypervisor: """ [client] rbd cache writethrough until flush = False rbd cache max dirty = 100663296 rbd cache size = 134217728 rbd cache target dirty = 50331648 """ How to reproduce: * create a virtual machine with a RBD backed disk (100GB or so) * install a linux distribution on it (we are using Ubuntu) * install fio (apt-get install fio) * run fio multiple times with (e.g.) the following test file: """ # This job file tries to mimic the Intel IOMeter File Server Access Pattern [global] description=Emulation of Intel IOmeter File Server Access Pattern randrepeat=0 filename=/root/test.dat # IOMeter defines the server loads as the following: # iodepth=1 Linear # iodepth=4 Very Light # iodepth=8 Light # iodepth=64Moderate # iodepth=256 Heavy iodepth=8 size=80g direct=0 ioengine=libaio [iometer] stonewall bs=4M rw=randrw [iometer_just_write] stonewall bs=4M rw=write [iometer_just_read] stonewall bs=4M rw=read """ You can measure the virtual machine RSS usage on the hypervisor with: virsh dommemstat | grep rss or if you are not using libvirt: grep RSS /proc//status When switching off the RBD client cache, all is ok again, as the process does not use so much memory anymore. There is already a ticket on the ceph bug tracker for this ([1]). However I can reproduce that memory behaviour only when using qemu (maybe it is using librbd in a special way?). Running directly 'fio' with the rbd engine does not result in that high memory usage. [1] http://tracker.ceph.com/issues/20054 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1701449/+subscriptions
[Qemu-devel] [PATCH] Add missing fp_access_check() to aarch64 crypto instructions
The aarch64 crypto instructions for AES and SHA are missing the
check for if the FPU is enabled.
Signed-off-by: Nick Reilly
---
target/arm/translate-a64.c | 12
1 file changed, 12 insertions(+)
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index e61bbd6..8105e7e 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -10929,6 +10929,10 @@ static void disas_crypto_aes(DisasContext *s, uint32_t
insn)
return;
}
+if (!fp_access_check(s)) {
+return;
+}
+
/* Note that we convert the Vx register indexes into the
* index within the vfp.regs[] array, so we can share the
* helper with the AArch32 instructions.
@@ -10993,6 +10997,10 @@ static void disas_crypto_three_reg_sha(DisasContext
*s, uint32_t insn)
return;
}
+if (!fp_access_check(s)) {
+return;
+}
+
tcg_rd_regno = tcg_const_i32(rd << 1);
tcg_rn_regno = tcg_const_i32(rn << 1);
tcg_rm_regno = tcg_const_i32(rm << 1);
@@ -11056,6 +11064,10 @@ static void disas_crypto_two_reg_sha(DisasContext *s,
uint32_t insn)
return;
}
+if (!fp_access_check(s)) {
+return;
+}
+
tcg_rd_regno = tcg_const_i32(rd << 1);
tcg_rn_regno = tcg_const_i32(rn << 1);
--
1.9.1
[Qemu-devel] vpc max table entries calculation error
i'm writing to discuss an issue in qemu's vpc creation.
when creating a dynamic-type vpc image with qemu-img like so:
$ qemu-img create -f vpc vhd.vhd 100M
Formatting 'vhd.vhd', fmt=vpc size=104857600
and then inspecting the file (with a tool i wrote; it's also easy to see in
a hex dump):
$ vhd-inspect ./vhd.vhd
MaxTableEntries: 51
BlockSize: 2097152
PhysicalSize: 104865792
VirtualSize: 104865792
PhysicalSize/BlockSize = 50
we see that the MaxTableEntries differs from the PhysicalSize/BlockSize.
in the vhd specification ([1] or [2]) we see that it says:
"Max Table Entries
This field holds the maximum entries present in the BAT. This should be
equal to the number of blocks in the disk (that is, the disk size divided
by the block size)."
however, in the QEMU function 'create_dynamic_disk' in block/vpc.c, we can
see there is one additional block added to the calculation for
num_bat_entries (same as MaxTableEntries):
num_bat_entries = (total_sectors + block_size / 512) / (block_size / 512);
so, i tried to fix this by removing the extra '+ block_size / 512'.
however, that seems to break some assumptions in 'vpc_open', namely this
code:
computed_size = (uint64_t) s->max_table_entries * s->block_size;
if (computed_size < bs->total_sectors * 512) {
error_setg(errp, "Page table too small");
ret = -EINVAL;
goto fail;
}
on the other hand, if i create the dynamic vpc using '-o force_size', the
disk size computation ends up slightly different, apparently due to not
using CHS, and the check passes.
so, i am not sure what the right fix is here, as it seems vpc is very
messy, but i do think that this is a bug because the incorrect
MaxTableEntries causes other tools to miscompute the real disk size. when
these dynamic-type vpcs with incorrect MaxTableEntries are converted to
fixed-type and uploaded to Microsoft Azure, it results in the hypervisor
rejecting the image.
does someone have an idea about the correct way to fix this?
[1] https://technet.microsoft.com/en-us/virtualization/bb676673.aspx
[2]
https://docs.google.com/document/d/1RWssryIPuH_5isISxu9cGisyOfAV8s1_-e-YhhiF-jY/edit
Re: [Qemu-devel] [PATCH] kvmclock: Ensure time in migration never goes backward
Hi all, On 06/05/14 08:16, Alexander Graf wrote: > > On 06.05.14 01:23, Marcelo Tosatti wrote: > >> 1) By what algorithm you retrieve >> and compare time in kvmclock guest structure and KVM_GET_CLOCK. >> What are the results of the comparison. >> And whether and backwards time was visible in the guest. > > I've managed to get my hands on a broken migration stream from Nick. > There I looked at the curr_clocksource structure and saw that the last > seen time on the kvmclock clock source was greater than the value that > the kvmclock device migrated. We've been seeing live migration failures where the guest sees time go backwards (= massive forward leap to the kernel, apparently) for a while now, affecting perhaps 5-10% of migrations we'd do (usually a large proportion of the migrations on a few hosts, rather than an even spread); initially in December, when we tried an upgrade to QEMU 1.7.1 and a 3.mumble (3.10?) kernel, from 1.5.0 and Debian's 3.2. My testing at the time seemed to indicate that either upgrade - qemu or kernel - caused the problems to show up. Guest symptoms are that the kernel enters a tight loop in __run_timers and stays there. In the end, I gave up and downgraded us again without any clear idea of what was happening, or why. In April, we finally got together a fairly reliable test case. This patch resolves the guest hangs in that test, and I've also been able to conduct > 1000 migrations of production guests without seeing the issue recur. So, Tested-by: Nick Thomas /Nick
Re: [Qemu-devel] AMD Opteron 6276 to Intel Xeon E5645 live migration failure
On 28/02/14 12:57, Paolo Bonzini wrote: > Il 28/02/2014 13:16, Nick Thomas ha scritto: >> I'd love to get this working, but I'm a little ignorant on where to >> begin, or even if it's possible at all. Are these CPUs just too old, or >> is a fixup missing in qemu (or kvm)? > > It's the latter (in kvm). Note that for migration to work, especially > for such different models, you have to disable CPU features that aren't > present in both models. In general the way to do this is to add "-cpu". > > I'd start debugging with "-cpu kvm64" on both sides. > > Paolo Sorry, I forgot to mention - most of my tests have been -cpu qemu64,-vmx,-svm. I've just re-run the minimal ones with that, and kvm64, for sanity. Same behaviour. In the interests of debugging, I guess the next step is to attach gdb to the destination and find out what's happening around the time the error messages are pushed out? I found an earlier thread ( http://www.spinics.net/lists/kvm/msg73478.html ) with a similar flavour that referenced a "vmxcap" tool - the output of that is here: Basic VMX Information Revision 15 VMCS size1024 VMCS restricted to 32 bit addresses no Dual-monitor support yes VMCS memory type 6 INS/OUTS instruction information yes IA32_VMX_TRUE_*_CTLS support yes pin-based controls External interrupt exiting yes NMI exiting yes Virtual NMIs yes Activate VMX-preemption timeryes Process posted interruptsno primary processor-based controls Interrupt window exiting yes Use TSC offsetting yes HLT exiting yes INVLPG exiting yes MWAIT exitingyes RDPMC exitingyes RDTSC exitingyes CR3-load exiting default CR3-store exitingdefault CR8-load exiting yes CR8-store exitingyes Use TPR shadow yes NMI-window exiting yes MOV-DR exiting yes Unconditional I/O exitingyes Use I/O bitmaps yes Monitor trap flagyes Use MSR bitmaps yes MONITOR exiting yes PAUSE exitingyes Activate secondary control yes secondary processor-based controls Virtualize APIC accesses yes Enable EPT yes Descriptor-table exiting yes Enable RDTSCPyes Virtualize x2APIC mode yes Enable VPID yes WBINVD exiting yes Unrestricted guest yes APIC register emulation no Virtual interrupt delivery no PAUSE-loop exiting yes RDRAND exiting no Enable INVPCID no Enable VM functions no VMCS shadowing no EPT-violation #VEno VM-Exit controls Save debug controls default Host address-space size yes Load IA32_PERF_GLOBAL_CTRL yes Acknowledge interrupt on exityes Save IA32_PATyes Load IA32_PATyes Save IA32_EFER yes Load IA32_EFER yes Save VMX-preemption timer value yes VM-Entry controls Load debug controls default IA-64 mode guest yes Entry to SMM yes Deactivate dual-monitor treatmentyes Load IA32_PERF_GLOBAL_CTRL yes Load IA32_PATyes Load IA32_EFER yes Miscellaneous data VMX-preemption timer scale (log2)7 Store EFER.LMA into IA-32e mode guest control yes HLT activity state yes Shutdown activity state yes Wait-for-SIPI activity state yes IA32_SMBASE support no Number of CR3-target values 4 MSR-load/store count recommenation 0 IA32_SMM_MONITOR_CTL[2] can be set to 1 no VMWRITE to VM-exit information fieldsno MSEG revision identifier 0 VPID and EPT capabilities Execute-only EPT translationsyes Page-wa
[Qemu-devel] AMD Opteron 6276 to Intel Xeon E5645 live migration failure
Hi there, We recently acquired some of the latter CPUs, and are attempting to add them to our existing cluster of AMD 6200/6300 KVM hosts. My life is made much easier if live migration between all hosts in the cluster works flawlessly, but I can reliably trigger a migration failure when moving a guest from the 6276 to the E5645 hosts. The failure looks like: KVM: injection failed, MSI lost (Operation not permitted) KVM: entry failed, hardware error 0x8021 If you're running a guest on an Intel machine without unrestricted mode support, the failure can be most likely due to the guest entering an invalid state for Intel VT. For example, the guest maybe running in big real mode which is not supported on less recent Intel processors. EAX= EBX=8160 ECX= EDX= ESI=0003 EDI=8160 EBP=8168f060 ESP=81601f10 EIP=8102b36c EFL=0246 [---Z-P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES = 9300 CS =f000 9b00 SS = 9300 DS = 9300 FS = 9300 GS = 9300 LDT= 8200 TR = 8b00 GDT= IDT= CR0=6010 CR2= CR3= CR4= DR0= DR1= DR2= DR3= DR6=0ff0 DR7=0400 EFER= Code=00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 <00> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 The guest itself is running Linux in ordinary 64-bit mode at the point where it's migrated. I also get this in dmesg: kvm: VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL does not work properly. Using workaround I've tried kernels 3.2.54 and 3.10.26 on the source host, and 3.2.54, 3.10.26 and 3.12.9 on the destination; and qemu versions 1.5.3 and 1.7.0. Same error in all cases. I've also tried twiddling the emulate_invalid_guest_state flag, to no effect. I can reproduce it with a guest as simple as: /opt/qemu-1.5.3/bin/qemu-system-x86_64 -enable-kvm -nographic -monitor stdio -kernel /boot/vmlinuz-3.10.26-bigv-20 -initrd /boot/initrd.img-3.10.26-bigv-20 -net none on the source, then migrating that to the destination. I can reliably boot and run the guest on the E5645 host, and migrate that guest to the AMD host. Migrating the same guest back again fails with the same error. Migrating from a later Intel machine (i7-4600U) to the earlier Intel seems to work fine. I'd love to get this working, but I'm a little ignorant on where to begin, or even if it's possible at all. Are these CPUs just too old, or is a fixup missing in qemu (or kvm)? /Nick
Re: [Qemu-devel] [PATCH 2/3] tests: add nbd-fault-injector.py utility
Hi,
On 25/02/14 10:09, Stefan Hajnoczi wrote:
> The nbd-fault-injector.py script is a special kind of NBD server. It
> throws away all writes and produces zeroes for reads. Given a list of
> fault injection rules, it can simulate NBD protocol errors and is useful
> for testing NBD client error handling code paths.
>
> See the patch for documentation. This scripts is modelled after Kevin
> Wolf 's blkdebug block driver.
>
> Signed-off-by: Stefan Hajnoczi
> ---
> tests/qemu-iotests/nbd-fault-injector.py | 231
> +++
> 1 file changed, 231 insertions(+)
> create mode 100755 tests/qemu-iotests/nbd-fault-injector.py
>
> diff --git a/tests/qemu-iotests/nbd-fault-injector.py
> b/tests/qemu-iotests/nbd-fault-injector.py
> new file mode 100755
> index 000..b4011f4
> --- /dev/null
> +++ b/tests/qemu-iotests/nbd-fault-injector.py
> @@ -0,0 +1,231 @@
> +#!/usr/bin/env python
> +# NBD server - fault injection utility
> +#
> +# Configuration file syntax:
> +# [inject-error "disconnect-neg1"]
> +# event=neg1
> +# io=readwrite
> +# when=before
> +#
> +# Note that Python's ConfigParser squashes together all sections with the
> same
> +# name, so give each [inject-error] a unique name.
> +#
> +# inject-error options:
> +# event - name of the trigger event
> +# "neg1" - first part of negotiation struct
> +# "export" - export struct
> +# "neg2" - second part of negotiation struct
> +# "request" - NBD request struct
> +# "reply" - NBD reply struct
> +# "data" - request/reply data
> +# io- I/O direction that triggers this rule:
> +# "read", "write", or "readwrite"
> +# default: readwrite
> +# when - when to inject the fault relative to the I/O operation
> +# "before" or "after"
> +# default: before
> +#
> +# Currently the only error injection action is to terminate the server
> process.
> +# This resets the TCP connection and thus forces the client to handle
> +# unexpected connection termination.
> +#
> +# Other error injection actions could be added in the future.
> +#
> +# Copyright Red Hat, Inc. 2014
> +#
> +# Authors:
> +# Stefan Hajnoczi
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +
> +import sys
> +import socket
> +import struct
> +import collections
> +import ConfigParser
> +
> +# Protocol constants
> +NBD_CMD_READ = 0
> +NBD_CMD_WRITE = 1
> +NBD_CMD_DISC = 2
> +NBD_REQUEST_MAGIC = 0x25609513
> +NBD_REPLY_MAGIC = 0x67446698
> +NBD_PASSWD = 0x4e42444d41474943
> +NBD_OPTS_MAGIC = 0x49484156454F5054
> +NBD_OPT_EXPORT_NAME = 1 << 0
> +
> +# Protocol structs
> +neg1_struct = struct.Struct('>QQH')
> +export_tuple = collections.namedtuple('Export', 'reserved magic opt len')
> +export_struct = struct.Struct('>IQII')
> +neg2_struct = struct.Struct('>QH124x')
> +request_tuple = collections.namedtuple('Request', 'magic type handle from_
> len')
> +request_struct = struct.Struct('>IIQQI')
> +reply_struct = struct.Struct('>IIQ')
> +
> +def err(msg):
> +sys.stderr.write(msg + '\n')
> +sys.exit(1)
> +
> +def recvall(sock, bufsize):
> +received = 0
> +chunks = []
> +while received < bufsize:
> +chunk = sock.recv(bufsize - received)
> +if len(chunk) == 0:
> +raise Exception('unexpected disconnect')
> +chunks.append(chunk)
> +received += len(chunk)
> +return ''.join(chunks)
> +
> +class Rule(object):
> +def __init__(self, name, event, io, when):
> +self.name = name
> +self.event = event
> +self.io = io
> +self.when = when
> +
> +def match(self, event, io, when):
> +if when != self.when:
> +return False
> +if event != self.event:
> +return False
> +if io != self.io and self.io != 'readwrite':
> +return False
> +return True
> +
> +class FaultInjectionSocket(object):
> +def __init__(self, sock, rules):
> +self.sock = sock
> +self.rules = rules
> +
> +def check(self, event, io, when):
> +for rule in self.rules:
> +if rule.match(event, io, when):
> +print 'Closing connection on rule match %s' % rule.name
> +sys.exit(0)
> +
> +def send(self, buf, event):
> +self.check(event, 'write', 'before')
> +self.sock.sendall(buf)
> +self.check(event, 'write', 'after')
> +
> +def recv(self, bufsize, event):
> +self.check(event, 'read', 'before')
> +data = recvall(self.sock, bufsize)
> +self.check(event, 'read', 'after')
> +return data
There's a class of error I recently encountered in our out-of-tree proxy
component that only shows up if a read or write is interrupted partway
through. Perhaps you could have a "during" event here that happens after
bufsize/2 bytes is written?
I've not
[Qemu-devel] [PATCH v2] tests: allow qemu-iotests to be run against nbd backend
From: Nick Thomas
To do this, we start a qemu-nbd process at _make_test_img and kill
it in _cleanup_test_img. $TEST_IMG is changed to point at the TCP
server. We also remove the checks for existence of binaries from
common.config - they're duplicated in common, and we can make the
qemu-nbd check conditional on $IMGPROTO being "nbd" if we do it there.
Signed-off-by: Nick Thomas
---
tests/qemu-iotests/common| 14 +++---
tests/qemu-iotests/common.config | 10 ++
tests/qemu-iotests/common.rc | 23 ++-
3 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 1f6fdf5..195722e 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -136,6 +136,7 @@ check options
-vmdk test vmdk
-rbdtest rbd
-sheepdog test sheepdog
+-nbdtest nbd
-xdiff graphical mode diff
-nocache use O_DIRECT on backing file
-misalign misalign memory allocations
@@ -197,12 +198,14 @@ testlist options
IMGPROTO=rbd
xpand=false
;;
-
-sheepdog)
IMGPROTO=sheepdog
xpand=false
;;
-
+-nbd)
+IMGPROTO=nbd
+xpand=false
+;;
-nocache)
QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS --nocache"
xpand=false
@@ -350,9 +353,14 @@ fi
[ "$QEMU" = "" ] && _fatal "qemu not found"
[ "$QEMU_IMG" = "" ] && _fatal "qemu-img not found"
-[ "$QEMU_IO" = "" ] && _fatal "qemu-img not found"
+[ "$QEMU_IO" = "" ] && _fatal "qemu-io not found"
+
+if [ "$IMGPROTO" = "nbd" ] ; then
+[ "$QEMU_NBD" = "" ] && _fatal "qemu-nbd not found"
+fi
if $valgrind; then
export REAL_QEMU_IO="$QEMU_IO_PROG"
export QEMU_IO_PROG=valgrind_qemu_io
fi
+
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index df082e7..08a3f10 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -90,21 +90,23 @@ export PS_ALL_FLAGS="-ef"
if [ -z "$QEMU_PROG" ]; then
export QEMU_PROG="`set_prog_path qemu`"
fi
-[ "$QEMU_PROG" = "" ] && _fatal "qemu not found"
if [ -z "$QEMU_IMG_PROG" ]; then
export QEMU_IMG_PROG="`set_prog_path qemu-img`"
fi
-[ "$QEMU_IMG_PROG" = "" ] && _fatal "qemu-img not found"
if [ -z "$QEMU_IO_PROG" ]; then
export QEMU_IO_PROG="`set_prog_path qemu-io`"
fi
-[ "$QEMU_IO_PROG" = "" ] && _fatal "qemu-io not found"
+
+if [ -z "$QEMU_NBD_PROG" ]; then
+export QEMU_NBD_PROG="`set_prog_path qemu-nbd`"
+fi
export QEMU=$QEMU_PROG
-export QEMU_IMG=$QEMU_IMG_PROG
+export QEMU_IMG=$QEMU_IMG_PROG
export QEMU_IO="$QEMU_IO_PROG $QEMU_IO_OPTIONS"
+export QEMU_NBD=$QEMU_NBD_PROG
[ -f /etc/qemu-iotest.config ] && . /etc/qemu-iotest.config
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 334534f..aef5f52 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -49,6 +49,9 @@ umask 022
if [ "$IMGPROTO" = "file" ]; then
TEST_IMG=$TEST_DIR/t.$IMGFMT
+elif [ "$IMGPROTO" = "nbd" ]; then
+TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
+TEST_IMG="nbd:127.0.0.1:10810"
else
TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
fi
@@ -86,6 +89,13 @@ _make_test_img()
local extra_img_options=""
local image_size=$*
local optstr=""
+local img_name=""
+
+if [ -n "$TEST_IMG_FILE" ]; then
+img_name=$TEST_IMG_FILE
+else
+img_name=$TEST_IMG
+fi
if [ -n "$IMGOPTS" ]; then
optstr=$(_optstr_add "$optstr" "$IMGOPTS")
@@ -104,7 +114,7 @@ _make_test_img()
fi
# XXX(hch): have global image options?
-$QEMU_IMG create -f $IMGFMT $extra_img_options $TEST_IMG $image_size | \
+$QEMU_IMG create -f $IMGFMT $extra_img_options $img_name $image_size | \
sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
-e "s#$TEST_DIR#TEST_DIR#g" \
-e "s#$IMGFMT#IMGFMT#g" \
@@ -115,12 +125,23 @@ _make_test_img()
-e "s# compat6=\\(on\\|off\\)##g" \
-e "s# static=\\(on\\|off\\)##g" \
-e "s# lazy_refcounts=\\(on\\|off\\)##g"
+
+# Start an NBD server on the image file, which is what we'll be talking to
+if [ $IMGPROTO = "nbd" ]; then
+eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 $TEST_IMG_FILE &"
+QEMU_NBD_PID=$!
+sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
+fi
}
_cleanup_test_img()
{
case "$IMGPROTO" in
+nbd)
+kill $QEMU_NBD_PID
+rm -f $TEST_IMG_FILE
+;;
file)
rm -f $TEST_DIR/t.$IMGFMT
rm -f $TEST_DIR/t.$IMGFMT.orig
--
1.7.2.5
[Qemu-devel] [PATCH] tests: allow qemu-iotests to be run against nbd backend
From: Nick Thomas
To do this, we start a qemu-nbd process at _make_test_img and kill
it in _cleanup_test_img. $TEST_IMG is changed to point at the TCP
server.
Signed-off-by: Nick Thomas
---
tests/qemu-iotests/common|7 +--
tests/qemu-iotests/common.config |8 +++-
tests/qemu-iotests/common.rc | 23 ++-
3 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 1f6fdf5..09dfdf1 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -136,6 +136,7 @@ check options
-vmdk test vmdk
-rbdtest rbd
-sheepdog test sheepdog
+-nbdtest nbd
-xdiff graphical mode diff
-nocache use O_DIRECT on backing file
-misalign misalign memory allocations
@@ -197,12 +198,14 @@ testlist options
IMGPROTO=rbd
xpand=false
;;
-
-sheepdog)
IMGPROTO=sheepdog
xpand=false
;;
-
+-nbd)
+IMGPROTO=nbd
+xpand=false
+;;
-nocache)
QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS --nocache"
xpand=false
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index df082e7..5383e4d 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -102,9 +102,15 @@ if [ -z "$QEMU_IO_PROG" ]; then
fi
[ "$QEMU_IO_PROG" = "" ] && _fatal "qemu-io not found"
+if [ -z "$QEMU_NBD_PROG" ]; then
+export QEMU_NBD_PROG="`set_prog_path qemu-nbd`"
+fi
+[ "$QEMU_IO_PROG" = "" ] && _fatal "qemu-io not found"
+
export QEMU=$QEMU_PROG
-export QEMU_IMG=$QEMU_IMG_PROG
+export QEMU_IMG=$QEMU_IMG_PROG
export QEMU_IO="$QEMU_IO_PROG $QEMU_IO_OPTIONS"
+export QEMU_NBD=$QEMU_NBD_PROG
[ -f /etc/qemu-iotest.config ] && . /etc/qemu-iotest.config
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 334534f..aef5f52 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -49,6 +49,9 @@ umask 022
if [ "$IMGPROTO" = "file" ]; then
TEST_IMG=$TEST_DIR/t.$IMGFMT
+elif [ "$IMGPROTO" = "nbd" ]; then
+TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
+TEST_IMG="nbd:127.0.0.1:10810"
else
TEST_IMG=$IMGPROTO:$TEST_DIR/t.$IMGFMT
fi
@@ -86,6 +89,13 @@ _make_test_img()
local extra_img_options=""
local image_size=$*
local optstr=""
+local img_name=""
+
+if [ -n "$TEST_IMG_FILE" ]; then
+img_name=$TEST_IMG_FILE
+else
+img_name=$TEST_IMG
+fi
if [ -n "$IMGOPTS" ]; then
optstr=$(_optstr_add "$optstr" "$IMGOPTS")
@@ -104,7 +114,7 @@ _make_test_img()
fi
# XXX(hch): have global image options?
-$QEMU_IMG create -f $IMGFMT $extra_img_options $TEST_IMG $image_size | \
+$QEMU_IMG create -f $IMGFMT $extra_img_options $img_name $image_size | \
sed -e "s#$IMGPROTO:$TEST_DIR#TEST_DIR#g" \
-e "s#$TEST_DIR#TEST_DIR#g" \
-e "s#$IMGFMT#IMGFMT#g" \
@@ -115,12 +125,23 @@ _make_test_img()
-e "s# compat6=\\(on\\|off\\)##g" \
-e "s# static=\\(on\\|off\\)##g" \
-e "s# lazy_refcounts=\\(on\\|off\\)##g"
+
+# Start an NBD server on the image file, which is what we'll be talking to
+if [ $IMGPROTO = "nbd" ]; then
+eval "$QEMU_NBD -v -t -b 127.0.0.1 -p 10810 $TEST_IMG_FILE &"
+QEMU_NBD_PID=$!
+sleep 1 # FIXME: qemu-nbd needs to be listening before we continue
+fi
}
_cleanup_test_img()
{
case "$IMGPROTO" in
+nbd)
+kill $QEMU_NBD_PID
+rm -f $TEST_IMG_FILE
+;;
file)
rm -f $TEST_DIR/t.$IMGFMT
rm -f $TEST_DIR/t.$IMGFMT.orig
--
1.7.2.5
[Qemu-devel] [PATCH 2/3] nbd: Explicitly disconnect and fail inflight I/O requests on error, then reconnect next I/O request.
The previous behaviour when an NBD connection broke was to fail just the broken
I/O request
and (sometimes) never unlock send_mutex. Now we explicitly call
nbd_teardown_connection and
fail all NBD requests in the "inflight" state - this allows werror/rerror
settings to be
applied to those requests all at once.
When a new request (or a request that was pending, but not yet inflight) is
issued against
the NBD driver, if we're not connected to the NBD server, we attempt to connect
(and fail
the new request immediately if that doesn't work). This isn't ideal, as it can
lead to many
failed attempts to connect to the NBD server in short order, but at least
allows us to get
over temporary failures in this state.
Signed-off-by: Nick Thomas
---
block/nbd.c | 72 --
1 files changed, 49 insertions(+), 23 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 9536408..1caae89 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -71,6 +71,9 @@ typedef struct BDRVNBDState {
char *host_spec;
} BDRVNBDState;
+static int nbd_establish_connection(BDRVNBDState *s);
+static void nbd_teardown_connection(BDRVNBDState *s, bool send_disconnect);
+
static bool nbd_is_connected(BDRVNBDState *s)
{
return s->sock >= 0;
@@ -152,6 +155,20 @@ static int nbd_have_request(void *opaque)
return s->in_flight > 0;
}
+static void nbd_abort_inflight_requests(BDRVNBDState *s)
+{
+int i;
+Coroutine *co;
+
+s->reply.handle = 0;
+for (i = 0; i < MAX_NBD_REQUESTS; i++) {
+co = s->recv_coroutine[i];
+if (co && co != qemu_coroutine_self()) {
+qemu_coroutine_enter(co, NULL);
+}
+}
+}
+
static void nbd_reply_ready(void *opaque)
{
BDRVNBDState *s = opaque;
@@ -168,8 +185,9 @@ static void nbd_reply_ready(void *opaque)
return;
}
if (ret < 0) {
-s->reply.handle = 0;
-goto fail;
+nbd_teardown_connection(s, false);
+nbd_abort_inflight_requests(s);
+return;
}
}
@@ -178,20 +196,15 @@ static void nbd_reply_ready(void *opaque)
* one coroutine is called until the reply finishes. */
i = HANDLE_TO_INDEX(s, s->reply.handle);
if (i >= MAX_NBD_REQUESTS) {
-goto fail;
+nbd_teardown_connection(s, false);
+nbd_abort_inflight_requests(s);
+return;
}
if (s->recv_coroutine[i]) {
qemu_coroutine_enter(s->recv_coroutine[i], NULL);
return;
}
-
-fail:
-for (i = 0; i < MAX_NBD_REQUESTS; i++) {
-if (s->recv_coroutine[i]) {
-qemu_coroutine_enter(s->recv_coroutine[i], NULL);
-}
-}
}
static void nbd_restart_write(void *opaque)
@@ -206,6 +219,13 @@ static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
int rc, ret;
qemu_co_mutex_lock(&s->send_mutex);
+
+if (!nbd_is_connected(s) && nbd_establish_connection(s) != 0) {
+nbd_abort_inflight_requests(s);
+qemu_co_mutex_unlock(&s->send_mutex);
+return -EIO;
+}
+
s->send_coroutine = qemu_coroutine_self();
qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write,
nbd_have_request, s);
@@ -214,6 +234,9 @@ static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov,
offset, request->len);
if (ret != request->len) {
+s->send_coroutine = NULL;
+nbd_teardown_connection(s, false);
+qemu_co_mutex_unlock(&s->send_mutex);
return -EIO;
}
}
@@ -259,9 +282,8 @@ static void nbd_coroutine_end(BDRVNBDState *s, struct nbd_request *request)
}
}
-static int nbd_establish_connection(BlockDriverState *bs)
+static int nbd_establish_connection(BDRVNBDState *s)
{
-BDRVNBDState *s = bs->opaque;
int sock;
int ret;
off_t size;
@@ -302,19 +324,23 @@ static int nbd_establish_connection(BlockDriverState *bs)
return 0;
}
-static void nbd_teardown_connection(BlockDriverState *bs)
+static void nbd_teardown_connection(BDRVNBDState *s, bool send_disconnect)
{
-BDRVNBDState *s = bs->opaque;
+
struct nbd_request request;
-request.type = NBD_CMD_DISC;
-request.from = 0;
-request.len = 0;
-nbd_send_request(s->sock, &request);
+if (nbd_is_connected(s)) {
+if (send_disconnect) {
+request.type = NBD_CMD_DISC;
+request.from = 0;
+request.len = 0;
+nbd_send_request(s->sock, &request);
+}
-qemu_aio_set_fd_handler(s->sock, NULL, NULL, NULL, NULL);
-closesocket(s->sock);
-s->sock = -1;
+qemu_aio_set_fd_handler(
[Qemu-devel] [PATCH 1/3] nbd: Only try to send flush/discard commands if connected to the NBD server
This is unlikely to come up now, but is a necessary prerequisite for
reconnection
behaviour.
Signed-off-by: Nick Thomas
---
block/nbd.c | 13 +++--
1 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 2bce47b..9536408 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -71,6 +71,11 @@ typedef struct BDRVNBDState {
char *host_spec;
} BDRVNBDState;
+static bool nbd_is_connected(BDRVNBDState *s)
+{
+return s->sock >= 0;
+}
+
static int nbd_config(BDRVNBDState *s, const char *filename, int flags)
{
char *file;
@@ -309,6 +314,7 @@ static void nbd_teardown_connection(BlockDriverState *bs)
qemu_aio_set_fd_handler(s->sock, NULL, NULL, NULL, NULL);
closesocket(s->sock);
+s->sock = -1;
}
static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
@@ -316,6 +322,8 @@ static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
BDRVNBDState *s = bs->opaque;
int result;
+s->sock = -1;
+
qemu_co_mutex_init(&s->send_mutex);
qemu_co_mutex_init(&s->free_sema);
@@ -431,7 +439,7 @@ static int nbd_co_flush(BlockDriverState *bs)
struct nbd_reply reply;
ssize_t ret;
-if (!(s->nbdflags & NBD_FLAG_SEND_FLUSH)) {
+if (!nbd_is_connected(s) || !(s->nbdflags & NBD_FLAG_SEND_FLUSH)) {
return 0;
}
@@ -462,7 +470,7 @@ static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
struct nbd_reply reply;
ssize_t ret;
-if (!(s->nbdflags & NBD_FLAG_SEND_TRIM)) {
+if (!nbd_is_connected(s) || !(s->nbdflags & NBD_FLAG_SEND_TRIM)) {
return 0;
}
request.type = NBD_CMD_TRIM;
@@ -515,3 +523,4 @@ static void bdrv_nbd_init(void)
}
block_init(bdrv_nbd_init);
+
[Qemu-devel] [PATCH 0/3] NBD reconnection behaviour
Hi all, This patchset is about making the NBD backend more useful. Currently, when the NBD server disconnects, the block device in the guest becomes unusable with no option to recover except to restart the QEMU process. These patches introduce a reconnect timer that fires every five seconds until we successfully reconnect. I/O requests that are inflight when the disconnection occurs, or requested while disconnected, are failed with an EIO - so the usual werror/rerror settings apply in those circumstances. All this means that, assuming you can get the NBD server up again, only some I/O requests are failed, rather than all of them. I've got a few more changes to make - specifically: * Allowing the reconnect timer delay to be configurable * Queuing and retrying I/O requests instead of EIO * Proactively killing the TCP connection if the server doesn't respond after a timeout. The rationale for the second is that some guests remount discs r/o if I/O requests fail (rather than apparently hang), which is a pain. The third allows us to quickly detect if a TCP connection disappears without a trace. However, I think these patches stand as an improvement on the curent situation, and I'd rather like some feedback on the best way to do the futher bits - assuming these patches get eventually accepted! Nick Thomas (3): nbd: Only try to send flush/discard commands if connected to the NBD server nbd: Explicitly disconnect and fail inflight I/O requests on error, then reconnect next I/O request. nbd: Move reconnection attempts from each new I/O request to a 5-second timer block/nbd.c | 117 +++ 1 files changed, 93 insertions(+), 24 deletions(-) -- 1.7.2.5
[Qemu-devel] [PATCH 3/3] nbd: Move reconnection attempts from each new I/O request to a 5-second timer
We don't want the guest to be able to control the reconnection rate (by sending
lots of I/O requests to the block device), so we try once every 5 seconds and
fail I/O requests that happen while we're disconnected.
TODO: Allow the reconnect delay to be specified as an option.
TODO: Queue, rather than fail, I/O requests when disconnected
Signed-off-by: Nick Thomas
---
block/nbd.c | 54 --
1 files changed, 44 insertions(+), 10 deletions(-)
diff --git a/block/nbd.c b/block/nbd.c
index 1caae89..e55a9a7 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -50,6 +50,8 @@
#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
#define INDEX_TO_HANDLE(bs, index) ((index) ^ ((uint64_t)(intptr_t)bs))
+#define RECONNECT_DELAY 5000
+
typedef struct BDRVNBDState {
int sock;
uint32_t nbdflags;
@@ -62,6 +64,8 @@ typedef struct BDRVNBDState {
Coroutine *send_coroutine;
int in_flight;
+QEMUTimer *recon_timer;
+
Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
struct nbd_reply reply;
@@ -125,6 +129,27 @@ out:
return err;
}
+static void nbd_reconnect(void *opaque)
+{
+BDRVNBDState *s = opaque;
+
+if (nbd_is_connected(s)) {
+logout("Already connected to NBD server, disarming timer\n");
+qemu_del_timer(s->recon_timer);
+return;
+}
+
+if (nbd_establish_connection(s) == 0) {
+logout("Reconnected to server, disabling reconnect timer\n");
+qemu_del_timer(s->recon_timer);
+} else {
+logout("Failed to reconnect, trying again in %ims\n", RECONNECT_DELAY);
+int64_t delay = qemu_get_clock_ms(rt_clock) + RECONNECT_DELAY;
+qemu_mod_timer(s->recon_timer, delay);
+}
+
+}
+
static void nbd_coroutine_start(BDRVNBDState *s, struct nbd_request *request)
{
int i;
@@ -155,11 +180,15 @@ static int nbd_have_request(void *opaque)
return s->in_flight > 0;
}
-static void nbd_abort_inflight_requests(BDRVNBDState *s)
+static void nbd_disconnect_for_reconnect(BDRVNBDState *s)
{
int i;
Coroutine *co;
+int64_t recon_time;
+nbd_teardown_connection(s, false);
+
+/* Abort all in-flight requests. TODO: these should be retried */
s->reply.handle = 0;
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
co = s->recv_coroutine[i];
@@ -167,6 +196,9 @@ static void nbd_abort_inflight_requests(BDRVNBDState *s)
qemu_coroutine_enter(co, NULL);
}
}
+
+recon_time = qemu_get_clock_ms(rt_clock) + RECONNECT_DELAY;
+qemu_mod_timer(s->recon_timer, recon_time);
}
static void nbd_reply_ready(void *opaque)
@@ -185,8 +217,7 @@ static void nbd_reply_ready(void *opaque)
return;
}
if (ret < 0) {
-nbd_teardown_connection(s, false);
-nbd_abort_inflight_requests(s);
+nbd_disconnect_for_reconnect(s);
return;
}
}
@@ -196,8 +227,7 @@ static void nbd_reply_ready(void *opaque)
* one coroutine is called until the reply finishes. */
i = HANDLE_TO_INDEX(s, s->reply.handle);
if (i >= MAX_NBD_REQUESTS) {
-nbd_teardown_connection(s, false);
-nbd_abort_inflight_requests(s);
+nbd_disconnect_for_reconnect(s);
return;
}
@@ -218,14 +248,14 @@ static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
{
int rc, ret;
-qemu_co_mutex_lock(&s->send_mutex);
-
-if (!nbd_is_connected(s) && nbd_establish_connection(s) != 0) {
-nbd_abort_inflight_requests(s);
-qemu_co_mutex_unlock(&s->send_mutex);
+/* TODO: We should be able to add this to the queue regardless, once
+ * coroutines retry if not connected */
+if (!nbd_is_connected(s)) {
return -EIO;
}
+qemu_co_mutex_lock(&s->send_mutex);
+
s->send_coroutine = qemu_coroutine_self();
qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write,
nbd_have_request, s);
@@ -349,6 +379,7 @@ static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
int result;
s->sock = -1;
+s->recon_timer = qemu_new_timer_ms(rt_clock, nbd_reconnect, s);
qemu_co_mutex_init(&s->send_mutex);
qemu_co_mutex_init(&s->free_sema);
@@ -520,6 +551,9 @@ static void nbd_close(BlockDriverState *bs)
g_free(s->export_name);
g_free(s->host_spec);
+qemu_del_timer(s->recon_timer);
+qemu_free_timer(s->recon_timer);
+
nbd_teardown_connection(s, true);
}
