[PATCH] accel/tcg: Add stub for probe_access()
The TCG helpers where added in b92e5a22ec3 in softmmu_template.h.
probe_write() was added in there in 3b4afc9e75a to be moved out
to accel/tcg/cputlb.c in 3b08f0a9254, and was later refactored
as probe_access() in c25c283df0f.
Since it is a TCG specific helper, add a stub to avoid failures
when building without TCG, such:
target/arm/helper.o: In function `probe_read':
include/exec/exec-all.h:345: undefined reference to `probe_access'
Signed-off-by: Philippe Mathieu-Daudé
---
Cc: Richard Henderson
Cc: Emilio G. Cota
Cc: Alex Bennée
Cc: David Hildenbrand
---
accel/stubs/tcg-stub.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
index 677191a69c..e4bbf997aa 100644
--- a/accel/stubs/tcg-stub.c
+++ b/accel/stubs/tcg-stub.c
@@ -22,3 +22,10 @@ void tb_flush(CPUState *cpu)
void tlb_set_dirty(CPUState *cpu, target_ulong vaddr)
{
}
+
+void *probe_access(CPUArchState *env, target_ulong addr, int size,
+ MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+ /* Handled by hardware accelerator. */
+ g_assert_not_reached();
+}
--
2.21.1
Re: [PATCH 2/6] target/arm: Make set_feature() available for other files
On 4/21/20 3:19 PM, Philippe Mathieu-Daudé wrote:
From: Thomas Huth
Move the common set_feature() and unset_feature() functions
from cpu.c and cpu64.c to cpu.h.
Suggested-by: Peter Maydell
Signed-off-by: Thomas Huth
Reviewed-by: Richard Henderson
Reviewed-by: Eric Auger
Message-ID: <20190921150420.30743-2-th...@redhat.com>
[PMD: Split Thomas's patch in two: set_feature, cpu_register (later)]
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu.h | 10 ++
target/arm/cpu.c | 10 --
target/arm/cpu64.c | 11 +--
3 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8b9f2961ba..5e32fe7518 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -696,6 +696,16 @@ typedef struct CPUARMState {
void *gicv3state;
} CPUARMState;
+static inline void set_feature(CPUARMState *env, int feature)
+{
+env->features |= 1ULL << feature;
+}
+
+static inline void unset_feature(CPUARMState *env, int feature)
+{
+env->features &= ~(1ULL << feature);
+}
Nack sigh... This still doesn't work:
target/arm/kvm64.c: At top level:
target/arm/kvm64.c:450:20: error: conflicting types for ‘set_feature’
static inline void set_feature(uint64_t *features, int feature)
^~~
In file included from include/sysemu/kvm.h:252:0,
from target/arm/kvm64.c:27:
target/arm/cpu.h:699:20: note: previous definition of ‘set_feature’ was here
static inline void set_feature(CPUARMState *env, int feature)
^~~
target/arm/kvm64.c:455:20: error: conflicting types for ‘unset_feature’
static inline void unset_feature(uint64_t *features, int feature)
^
In file included from include/sysemu/kvm.h:252:0,
from target/arm/kvm64.c:27:
target/arm/cpu.h:704:20: note: previous definition of ‘unset_feature’
was here
static inline void unset_feature(CPUARMState *env, int feature)
^
rules.mak:69: recipe for target 'target/arm/kvm64.o' failed
make: *** [target/arm/kvm64.o] Error 1
+
/**
* ARMELChangeHookFn:
* type of a function which can be registered via
arm_register_el_change_hook()
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a79f233b17..37f18d1648 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -724,16 +724,6 @@ static bool arm_cpu_virtio_is_big_endian(CPUState *cs)
#endif
-static inline void set_feature(CPUARMState *env, int feature)
-{
-env->features |= 1ULL << feature;
-}
-
-static inline void unset_feature(CPUARMState *env, int feature)
-{
-env->features &= ~(1ULL << feature);
-}
-
static int
print_insn_thumb1(bfd_vma pc, disassemble_info *info)
{
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 62d36f9e8d..622082eae2 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -21,6 +21,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "cpu.h"
+#include "internals.h"
This include is not necessary.
#include "qemu/module.h"
#if !defined(CONFIG_USER_ONLY)
#include "hw/loader.h"
@@ -29,16 +30,6 @@
#include "kvm_arm.h"
#include "qapi/visitor.h"
-static inline void set_feature(CPUARMState *env, int feature)
-{
-env->features |= 1ULL << feature;
-}
-
-static inline void unset_feature(CPUARMState *env, int feature)
-{
-env->features &= ~(1ULL << feature);
-}
-
#ifndef CONFIG_USER_ONLY
static uint64_t a57_a53_l2ctlr_read(CPUARMState *env, const ARMCPRegInfo *ri)
{
RE: [PATCH 1/3] net/colo-compare.c: Create event_bh with the right AioContext
> -Original Message-
> From: Lukas Straub
> Sent: Wednesday, April 22, 2020 5:40 PM
> To: Zhang, Chen
> Cc: qemu-devel ; Li Zhijian
> ; Jason Wang ; Marc-
> André Lureau ; Paolo Bonzini
>
> Subject: Re: [PATCH 1/3] net/colo-compare.c: Create event_bh with the right
> AioContext
>
> On Wed, 22 Apr 2020 09:03:00 +
> "Zhang, Chen" wrote:
>
> > > -Original Message-
> > > From: Lukas Straub
> > > Sent: Wednesday, April 22, 2020 4:43 PM
> > > To: Zhang, Chen
> > > Cc: qemu-devel ; Li Zhijian
> > > ; Jason Wang ; Marc-
> > > André Lureau ; Paolo Bonzini
> > >
> > > Subject: Re: [PATCH 1/3] net/colo-compare.c: Create event_bh with
> > > the right AioContext
> > >
> > > On Wed, 22 Apr 2020 08:29:39 +
> > > "Zhang, Chen" wrote:
> > >
> > > > > -Original Message-
> > > > > From: Lukas Straub
> > > > > Sent: Thursday, April 9, 2020 2:34 AM
> > > > > To: qemu-devel
> > > > > Cc: Zhang, Chen ; Li Zhijian
> > > > > ; Jason Wang ;
> > > > > Marc- André Lureau ; Paolo Bonzini
> > > > >
> > > > > Subject: [PATCH 1/3] net/colo-compare.c: Create event_bh with
> > > > > the right AioContext
> > > > >
> > > > > qemu_bh_new will set the bh to be executed in the main loop.
> > > > > This causes problems as colo_compare_handle_event assumes that
> > > > > it has exclusive access the queues, which are also accessed in
> > > > > the iothread. It also assumes that it runs in a different thread
> > > > > than the caller and takes the appropriate locks.
> > > > >
> > > > > Create the bh with the AioContext of the iothread to fulfill
> > > > > these assumptions.
> > > > >
> > > >
> > > > Looks good for me, I assume it will increase performance. Do you
> > > > have
> > > related data?
> > >
> > > No, this fixes several crashes because the queues where accessed
> > > concurrently from multiple threads. Sorry for my bad wording.
> >
> > Can you describe some details about the crash? Step by step?
> > Maybe I can re-produce and test it for this patch.
>
> There is no clear test case. For me the crashes happened after 1-20h of
> runtime with lots of checkpoints (800ms) and some network traffic. The
> coredump always showed that two threads where doing operations on the
> queues simultaneously.
> Unfortunately, I don't have the coredumps anymore.
OK, Although I have not encountered the problem you described.
I have test this patch, looks running fine.
Reviewed-by: Zhang Chen
Thanks
Zhang Chen
>
> Regards,
> Lukas Straub
>
> > Thanks
> > Zhang Chen
> >
> > >
> > > Regards,
> > > Lukas Straub
> > >
> > > > Thanks
> > > > Zhang Chen
> > > >
> > > > > Signed-off-by: Lukas Straub
> > > > > ---
> > > > > net/colo-compare.c | 3 ++-
> > > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > > > > 10c0239f9d..1de4220fe2 100644
> > > > > --- a/net/colo-compare.c
> > > > > +++ b/net/colo-compare.c
> > > > > @@ -890,6 +890,7 @@ static void colo_compare_handle_event(void
> > > > > *opaque)
> > > > >
> > > > > static void colo_compare_iothread(CompareState *s) {
> > > > > +AioContext *ctx = iothread_get_aio_context(s->iothread);
> > > > > object_ref(OBJECT(s->iothread));
> > > > > s->worker_context =
> > > > > iothread_get_g_main_context(s->iothread);
> > > > >
> > > > > @@ -906,7 +907,7 @@ static void
> > > > > colo_compare_iothread(CompareState
> > > *s)
> > > > > }
> > > > >
> > > > > colo_compare_timer_init(s);
> > > > > -s->event_bh = qemu_bh_new(colo_compare_handle_event, s);
> > > > > +s->event_bh = aio_bh_new(ctx, colo_compare_handle_event,
> > > > > + s);
> > > > > }
> > > > >
> > > > > static char *compare_get_pri_indev(Object *obj, Error **errp)
> > > > > --
> > > > > 2.20.1
> > > >
> >
RE: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up
Hi Jason, Please review this series when you free. Thanks Zhang Chen > -Original Message- > From: Zhang, Chen > Sent: Saturday, April 11, 2020 11:38 AM > To: Jason Wang ; qemu-dev de...@nongnu.org> > Cc: Zhang Chen ; Zhang, Chen > > Subject: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to > users and clean up > > From: Zhang Chen > > This series make a way to config COLO "max_queue_size" parameters > according to user's scenarios and environments and do some clean up for > descriptions. > > Zhang Chen (2): > net/colo-compare.c: Expose compare "max_queue_size" to users > qemu-options.hx: Clean up and fix typo for colo-compare > > net/colo-compare.c | 43 > ++- > qemu-options.hx| 33 + > 2 files changed, 59 insertions(+), 17 deletions(-) > > -- > 2.17.1
[PATCH v2 3/5] target/arm/cpu: Use ARRAY_SIZE() to iterate over ARMCPUInfo[]
Suggested-by: Richard Henderson
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu.c | 8 +++-
target/arm/cpu64.c | 8 +++-
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 47e35400da..30e961f775 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2745,7 +2745,6 @@ static const ARMCPUInfo arm_cpus[] = {
{ .name = "any", .initfn = arm_max_initfn },
#endif
#endif
-{ .name = NULL }
};
static Property arm_cpu_properties[] = {
@@ -2893,14 +2892,13 @@ static const TypeInfo idau_interface_type_info = {
static void arm_cpu_register_types(void)
{
-const ARMCPUInfo *info = arm_cpus;
+size_t i;
type_register_static(&arm_cpu_type_info);
type_register_static(&idau_interface_type_info);
-while (info->name) {
-arm_cpu_register(info);
-info++;
+for (i = 0; i < ARRAY_SIZE(arm_cpus); ++i) {
+arm_cpu_register(&arm_cpus[i]);
}
#ifdef CONFIG_KVM
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 951588c56e..ef9231d55b 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -742,7 +742,6 @@ static const ARMCPUInfo aarch64_cpus[] = {
{ .name = "cortex-a53", .initfn = aarch64_a53_initfn },
{ .name = "cortex-a72", .initfn = aarch64_a72_initfn },
{ .name = "max",.initfn = aarch64_max_initfn },
-{ .name = NULL }
};
static bool aarch64_cpu_get_aarch64(Object *obj, Error **errp)
@@ -848,13 +847,12 @@ static const TypeInfo aarch64_cpu_type_info = {
static void aarch64_cpu_register_types(void)
{
-const ARMCPUInfo *info = aarch64_cpus;
+size_t i;
type_register_static(&aarch64_cpu_type_info);
-while (info->name) {
-aarch64_cpu_register(info);
-info++;
+for (i = 0; i < ARRAY_SIZE(aarch64_cpus); ++i) {
+aarch64_cpu_register(&aarch64_cpus[i]);
}
}
--
2.21.1
[PATCH v2 1/5] target/arm: Restric the Address Translate write operation to TCG accel
Under KVM these registers are written by the hardware.
Restrict the writefn handlers to TCG to avoid when building
without TCG:
LINKaarch64-softmmu/qemu-system-aarch64
target/arm/helper.o: In function `do_ats_write':
target/arm/helper.c:3524: undefined reference to `raise_exception'
Suggested-by: Richard Henderson
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/helper.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 7e9ea5d20f..dfefb9b3d9 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3442,6 +3442,7 @@ static CPAccessResult ats_access(CPUARMState *env, const
ARMCPRegInfo *ri,
return CP_ACCESS_OK;
}
+#ifdef CONFIG_TCG
static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
MMUAccessType access_type, ARMMMUIdx mmu_idx)
{
@@ -3602,9 +3603,11 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t
value,
}
return par64;
}
+#endif /* CONFIG_TCG */
static void ats_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
{
+#ifdef CONFIG_TCG
MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
uint64_t par64;
ARMMMUIdx mmu_idx;
@@ -3664,17 +3667,26 @@ static void ats_write(CPUARMState *env, const
ARMCPRegInfo *ri, uint64_t value)
par64 = do_ats_write(env, value, access_type, mmu_idx);
A32_BANKED_CURRENT_REG_SET(env, par, par64);
+#else
+/* Handled by hardware accelerator. */
+g_assert_not_reached();
+#endif /* CONFIG_TCG */
}
static void ats1h_write(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
+#ifdef CONFIG_TCG
MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
uint64_t par64;
par64 = do_ats_write(env, value, access_type, ARMMMUIdx_E2);
A32_BANKED_CURRENT_REG_SET(env, par, par64);
+#else
+/* Handled by hardware accelerator. */
+g_assert_not_reached();
+#endif /* CONFIG_TCG */
}
static CPAccessResult at_s1e2_access(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -3689,6 +3701,7 @@ static CPAccessResult at_s1e2_access(CPUARMState *env,
const ARMCPRegInfo *ri,
static void ats_write64(CPUARMState *env, const ARMCPRegInfo *ri,
uint64_t value)
{
+#ifdef CONFIG_TCG
MMUAccessType access_type = ri->opc2 & 1 ? MMU_DATA_STORE : MMU_DATA_LOAD;
ARMMMUIdx mmu_idx;
int secure = arm_is_secure_below_el3(env);
@@ -3728,6 +3741,10 @@ static void ats_write64(CPUARMState *env, const
ARMCPRegInfo *ri,
}
env->cp15.par_el[1] = do_ats_write(env, value, access_type, mmu_idx);
+#else
+/* Handled by hardware accelerator. */
+g_assert_not_reached();
+#endif /* CONFIG_TCG */
}
#endif
--
2.21.1
[PATCH v2 0/5] target/arm: Restrict TCG cpus to TCG accel
These are the uncontroversial patches from "Support disabling TCG on ARM (part 2)" https://www.mail-archive.com/qemu-devel@nongnu.org/msg689168.html The other patches are blocked by the "accel: Allow targets to use Kconfig" series: https://www.mail-archive.com/qemu-devel@nongnu.org/msg689024.html All patches reviewed. Since v1: - Dropped 'Make set_feature() available for other files' patch which fails to build with KVM only, see: https://lists.gnu.org/archive/html/qemu-devel/2020-04/msg03843.html Many thanks to Richard Henderson for his patience! Regards, Phil. Philippe Mathieu-Daudé (4): target/arm: Restric the Address Translate write operation to TCG accel target/arm/cpu: Use ARRAY_SIZE() to iterate over ARMCPUInfo[] target/arm/cpu: Update coding style to make checkpatch.pl happy target/arm: Restrict TCG cpus to TCG accel Thomas Huth (1): target/arm: Make cpu_register() available for other files target/arm/cpu-qom.h | 9 +- target/arm/cpu.c | 647 +- target/arm/cpu64.c | 16 +- target/arm/cpu_tcg.c | 664 +++ target/arm/helper.c | 17 + target/arm/Makefile.objs | 1 + 6 files changed, 698 insertions(+), 656 deletions(-) create mode 100644 target/arm/cpu_tcg.c -- 2.21.1
[PATCH v2 2/5] target/arm: Make cpu_register() available for other files
From: Thomas Huth
Make cpu_register() (renamed to arm_cpu_register()) available
from internals.h so we can register CPUs also from other files
in the future.
Signed-off-by: Thomas Huth
Reviewed-by: Richard Henderson
Reviewed-by: Eric Auger
Message-ID: <20190921150420.30743-2-th...@redhat.com>
[PMD: Only take cpu_register() from Thomas's patch]
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu-qom.h | 9 -
target/arm/cpu.c | 10 ++
target/arm/cpu64.c | 8 +---
3 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/target/arm/cpu-qom.h b/target/arm/cpu-qom.h
index d95568bf05..56395b87f6 100644
--- a/target/arm/cpu-qom.h
+++ b/target/arm/cpu-qom.h
@@ -35,7 +35,14 @@ struct arm_boot_info;
#define TYPE_ARM_MAX_CPU "max-" TYPE_ARM_CPU
-typedef struct ARMCPUInfo ARMCPUInfo;
+typedef struct ARMCPUInfo {
+const char *name;
+void (*initfn)(Object *obj);
+void (*class_init)(ObjectClass *oc, void *data);
+} ARMCPUInfo;
+
+void arm_cpu_register(const ARMCPUInfo *info);
+void aarch64_cpu_register(const ARMCPUInfo *info);
/**
* ARMCPUClass:
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a79f233b17..47e35400da 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2693,12 +2693,6 @@ static void arm_max_initfn(Object *obj)
#endif /* !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64) */
-struct ARMCPUInfo {
-const char *name;
-void (*initfn)(Object *obj);
-void (*class_init)(ObjectClass *oc, void *data);
-};
-
static const ARMCPUInfo arm_cpus[] = {
#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
{ .name = "arm926", .initfn = arm926_initfn },
@@ -2864,7 +2858,7 @@ static void cpu_register_class_init(ObjectClass *oc, void
*data)
acc->info = data;
}
-static void cpu_register(const ARMCPUInfo *info)
+void arm_cpu_register(const ARMCPUInfo *info)
{
TypeInfo type_info = {
.parent = TYPE_ARM_CPU,
@@ -2905,7 +2899,7 @@ static void arm_cpu_register_types(void)
type_register_static(&idau_interface_type_info);
while (info->name) {
-cpu_register(info);
+arm_cpu_register(info);
info++;
}
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 62d36f9e8d..951588c56e 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -737,12 +737,6 @@ static void aarch64_max_initfn(Object *obj)
cpu_max_set_sve_max_vq, NULL, NULL, &error_fatal);
}
-struct ARMCPUInfo {
-const char *name;
-void (*initfn)(Object *obj);
-void (*class_init)(ObjectClass *oc, void *data);
-};
-
static const ARMCPUInfo aarch64_cpus[] = {
{ .name = "cortex-a57", .initfn = aarch64_a57_initfn },
{ .name = "cortex-a53", .initfn = aarch64_a53_initfn },
@@ -825,7 +819,7 @@ static void cpu_register_class_init(ObjectClass *oc, void
*data)
acc->info = data;
}
-static void aarch64_cpu_register(const ARMCPUInfo *info)
+void aarch64_cpu_register(const ARMCPUInfo *info)
{
TypeInfo type_info = {
.parent = TYPE_AARCH64_CPU,
--
2.21.1
[PATCH v2 5/5] target/arm: Restrict TCG cpus to TCG accel
A KVM-only build won't be able to run TCG cpus.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu.c | 634 -
target/arm/cpu_tcg.c | 664 +++
target/arm/Makefile.objs | 1 +
3 files changed, 665 insertions(+), 634 deletions(-)
create mode 100644 target/arm/cpu_tcg.c
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a1e38b38ba..fed9ccf6d5 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -574,32 +574,6 @@ bool arm_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
return true;
}
-#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
-static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
-{
-CPUClass *cc = CPU_GET_CLASS(cs);
-ARMCPU *cpu = ARM_CPU(cs);
-CPUARMState *env = &cpu->env;
-bool ret = false;
-
-/*
- * ARMv7-M interrupt masking works differently than -A or -R.
- * There is no FIQ/IRQ distinction. Instead of I and F bits
- * masking FIQ and IRQ interrupts, an exception is taken only
- * if it is higher priority than the current execution priority
- * (which depends on state like BASEPRI, FAULTMASK and the
- * currently active exception).
- */
-if (interrupt_request & CPU_INTERRUPT_HARD
-&& (armv7m_nvic_can_take_pending_exception(env->nvic))) {
-cs->exception_index = EXCP_IRQ;
-cc->do_interrupt(cs);
-ret = true;
-}
-return ret;
-}
-#endif
-
void arm_cpu_update_virq(ARMCPU *cpu)
{
/*
@@ -1830,406 +1804,6 @@ static ObjectClass *arm_cpu_class_by_name(const char
*cpu_model)
/* CPU models. These are not needed for the AArch64 linux-user build. */
#if !defined(CONFIG_USER_ONLY) || !defined(TARGET_AARCH64)
-static void arm926_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-
-cpu->dtb_compatible = "arm,arm926";
-set_feature(&cpu->env, ARM_FEATURE_V5);
-set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
-set_feature(&cpu->env, ARM_FEATURE_CACHE_TEST_CLEAN);
-cpu->midr = 0x41069265;
-cpu->reset_fpsid = 0x41011090;
-cpu->ctr = 0x1dd20d2;
-cpu->reset_sctlr = 0x00090078;
-
-/*
- * ARMv5 does not have the ID_ISAR registers, but we can still
- * set the field to indicate Jazelle support within QEMU.
- */
-cpu->isar.id_isar1 = FIELD_DP32(cpu->isar.id_isar1, ID_ISAR1, JAZELLE, 1);
-/*
- * Similarly, we need to set MVFR0 fields to enable vfp and short vector
- * support even though ARMv5 doesn't have this register.
- */
-cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPSHVEC, 1);
-cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPSP, 1);
-cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPDP, 1);
-}
-
-static void arm946_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-
-cpu->dtb_compatible = "arm,arm946";
-set_feature(&cpu->env, ARM_FEATURE_V5);
-set_feature(&cpu->env, ARM_FEATURE_PMSA);
-set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
-cpu->midr = 0x41059461;
-cpu->ctr = 0x0f004006;
-cpu->reset_sctlr = 0x0078;
-}
-
-static void arm1026_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-
-cpu->dtb_compatible = "arm,arm1026";
-set_feature(&cpu->env, ARM_FEATURE_V5);
-set_feature(&cpu->env, ARM_FEATURE_AUXCR);
-set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
-set_feature(&cpu->env, ARM_FEATURE_CACHE_TEST_CLEAN);
-cpu->midr = 0x4106a262;
-cpu->reset_fpsid = 0x410110a0;
-cpu->ctr = 0x1dd20d2;
-cpu->reset_sctlr = 0x00090078;
-cpu->reset_auxcr = 1;
-
-/*
- * ARMv5 does not have the ID_ISAR registers, but we can still
- * set the field to indicate Jazelle support within QEMU.
- */
-cpu->isar.id_isar1 = FIELD_DP32(cpu->isar.id_isar1, ID_ISAR1, JAZELLE, 1);
-/*
- * Similarly, we need to set MVFR0 fields to enable vfp and short vector
- * support even though ARMv5 doesn't have this register.
- */
-cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPSHVEC, 1);
-cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPSP, 1);
-cpu->isar.mvfr0 = FIELD_DP32(cpu->isar.mvfr0, MVFR0, FPDP, 1);
-
-{
-/* The 1026 had an IFAR at c6,c0,0,1 rather than the ARMv6 c6,c0,0,2 */
-ARMCPRegInfo ifar = {
-.name = "IFAR", .cp = 15, .crn = 6, .crm = 0, .opc1 = 0, .opc2 = 1,
-.access = PL1_RW,
-.fieldoffset = offsetof(CPUARMState, cp15.ifar_ns),
-.resetvalue = 0
-};
-define_one_arm_cp_reg(cpu, &ifar);
-}
-}
-
-static void arm1136_r2_initfn(Object *obj)
-{
-ARMCPU *cpu = ARM_CPU(obj);
-/*
- * What qemu calls "arm1136_r2" is actually the 1136 r0p2, ie an
- * older core than plain "arm1136". In particular this does not
- * have the v6K features.
- * These ID register values are correct for 1136 but may be wrong
- *
[PATCH v2 4/5] target/arm/cpu: Update coding style to make checkpatch.pl happy
We will move this code in the next commit. Clean it up
first to avoid checkpatch.pl errors.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 30e961f775..a1e38b38ba 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -582,7 +582,8 @@ static bool arm_v7m_cpu_exec_interrupt(CPUState *cs, int
interrupt_request)
CPUARMState *env = &cpu->env;
bool ret = false;
-/* ARMv7-M interrupt masking works differently than -A or -R.
+/*
+ * ARMv7-M interrupt masking works differently than -A or -R.
* There is no FIQ/IRQ distinction. Instead of I and F bits
* masking FIQ and IRQ interrupts, an exception is taken only
* if it is higher priority than the current execution priority
@@ -1912,7 +1913,8 @@ static void arm1026_initfn(Object *obj)
static void arm1136_r2_initfn(Object *obj)
{
ARMCPU *cpu = ARM_CPU(obj);
-/* What qemu calls "arm1136_r2" is actually the 1136 r0p2, ie an
+/*
+ * What qemu calls "arm1136_r2" is actually the 1136 r0p2, ie an
* older core than plain "arm1136". In particular this does not
* have the v6K features.
* These ID register values are correct for 1136 but may be wrong
@@ -2698,7 +2700,8 @@ static const ARMCPUInfo arm_cpus[] = {
{ .name = "arm926", .initfn = arm926_initfn },
{ .name = "arm946", .initfn = arm946_initfn },
{ .name = "arm1026", .initfn = arm1026_initfn },
-/* What QEMU calls "arm1136-r2" is actually the 1136 r0p2, i.e. an
+/*
+ * What QEMU calls "arm1136-r2" is actually the 1136 r0p2, i.e. an
* older core than plain "arm1136". In particular this does not
* have the v6K features.
*/
--
2.21.1
Re: [PATCH] accel/tcg: Add stub for probe_access()
On 23.04.20 09:10, Philippe Mathieu-Daudé wrote:
> The TCG helpers where added in b92e5a22ec3 in softmmu_template.h.
> probe_write() was added in there in 3b4afc9e75a to be moved out
> to accel/tcg/cputlb.c in 3b08f0a9254, and was later refactored
> as probe_access() in c25c283df0f.
> Since it is a TCG specific helper, add a stub to avoid failures
> when building without TCG, such:
>
> target/arm/helper.o: In function `probe_read':
> include/exec/exec-all.h:345: undefined reference to `probe_access'
I think you're missing the most important commit:
0d57b4999220 ("target/arm: Add support for DC CVAP & DC CVADP ins")
I do wonder if dccvap_writefn() and calling code should be compiled for
TCG only (CONFIG_TCG). I assume it is only called from TCG code -
otherwise it would already be semi-broken.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Cc: Richard Henderson
> Cc: Emilio G. Cota
> Cc: Alex Bennée
> Cc: David Hildenbrand
> ---
> accel/stubs/tcg-stub.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
> index 677191a69c..e4bbf997aa 100644
> --- a/accel/stubs/tcg-stub.c
> +++ b/accel/stubs/tcg-stub.c
> @@ -22,3 +22,10 @@ void tb_flush(CPUState *cpu)
> void tlb_set_dirty(CPUState *cpu, target_ulong vaddr)
> {
> }
> +
> +void *probe_access(CPUArchState *env, target_ulong addr, int size,
> + MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> +{
> + /* Handled by hardware accelerator. */
> + g_assert_not_reached();
> +}
>
Still, this makes sense to me as well
Reviewed-by: David Hildenbrand
--
Thanks,
David / dhildenb
Re: [PATCH 01/11] MAINTAINERS: Fix KVM path expansion glob
On 3/16/20 7:30 PM, Richard Henderson wrote:
On 3/16/20 5:00 AM, Philippe Mathieu-Daudé wrote:
The KVM files has been moved from target-ARCH to the target/ARCH/
folder in commit fcf5ef2a. Fix the pathname expansion.
Fixes: fcf5ef2a ("Move target-* CPU file into a target/ folder")
Signed-off-by: Philippe Mathieu-Daudé
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Richard Henderson
Paolo, do you plan to queue this trivial patch or should I ask qemu-trivial?
Re: [PATCH] accel/tcg: Add stub for probe_access()
On Thu, Apr 23, 2020 at 9:49 AM David Hildenbrand wrote:
>
> On 23.04.20 09:10, Philippe Mathieu-Daudé wrote:
> > The TCG helpers where added in b92e5a22ec3 in softmmu_template.h.
> > probe_write() was added in there in 3b4afc9e75a to be moved out
> > to accel/tcg/cputlb.c in 3b08f0a9254, and was later refactored
> > as probe_access() in c25c283df0f.
> > Since it is a TCG specific helper, add a stub to avoid failures
> > when building without TCG, such:
> >
> > target/arm/helper.o: In function `probe_read':
> > include/exec/exec-all.h:345: undefined reference to `probe_access'
>
> I think you're missing the most important commit:
>
> 0d57b4999220 ("target/arm: Add support for DC CVAP & DC CVADP ins")
>
> I do wonder if dccvap_writefn() and calling code should be compiled for
> TCG only (CONFIG_TCG). I assume it is only called from TCG code -
> otherwise it would already be semi-broken.
I can only recommend you to read the thread after this previous patch,
as I don't have the knowledge to explain...:
https://www.mail-archive.com/qemu-devel@nongnu.org/msg689115.html
>
> >
> > Signed-off-by: Philippe Mathieu-Daudé
> > ---
> > Cc: Richard Henderson
> > Cc: Emilio G. Cota
> > Cc: Alex Bennée
> > Cc: David Hildenbrand
> > ---
> > accel/stubs/tcg-stub.c | 7 +++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/accel/stubs/tcg-stub.c b/accel/stubs/tcg-stub.c
> > index 677191a69c..e4bbf997aa 100644
> > --- a/accel/stubs/tcg-stub.c
> > +++ b/accel/stubs/tcg-stub.c
> > @@ -22,3 +22,10 @@ void tb_flush(CPUState *cpu)
> > void tlb_set_dirty(CPUState *cpu, target_ulong vaddr)
> > {
> > }
> > +
> > +void *probe_access(CPUArchState *env, target_ulong addr, int size,
> > + MMUAccessType access_type, int mmu_idx, uintptr_t
> > retaddr)
> > +{
> > + /* Handled by hardware accelerator. */
> > + g_assert_not_reached();
> > +}
> >
>
> Still, this makes sense to me as well
>
> Reviewed-by: David Hildenbrand
>
> --
> Thanks,
>
> David / dhildenb
>
Re: [PATCH] accel/tcg: Add stub for probe_access()
On 23.04.20 09:59, Philippe Mathieu-Daudé wrote:
> On Thu, Apr 23, 2020 at 9:49 AM David Hildenbrand wrote:
>>
>> On 23.04.20 09:10, Philippe Mathieu-Daudé wrote:
>>> The TCG helpers where added in b92e5a22ec3 in softmmu_template.h.
>>> probe_write() was added in there in 3b4afc9e75a to be moved out
>>> to accel/tcg/cputlb.c in 3b08f0a9254, and was later refactored
>>> as probe_access() in c25c283df0f.
>>> Since it is a TCG specific helper, add a stub to avoid failures
>>> when building without TCG, such:
>>>
>>> target/arm/helper.o: In function `probe_read':
>>> include/exec/exec-all.h:345: undefined reference to `probe_access'
>>
>> I think you're missing the most important commit:
>>
>> 0d57b4999220 ("target/arm: Add support for DC CVAP & DC CVADP ins")
>>
>> I do wonder if dccvap_writefn() and calling code should be compiled for
>> TCG only (CONFIG_TCG). I assume it is only called from TCG code -
>> otherwise it would already be semi-broken.
>
> I can only recommend you to read the thread after this previous patch,
> as I don't have the knowledge to explain...:
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg689115.html
Yeah, me neither. Sounds wrong to me to have TCG-only code stick around
in !CONFIG_TCG builds. But I am pretty sure ARM people know what they
are doing.
--
Thanks,
David / dhildenb
Re: [PATCH v21 QEMU 4/5] virtio-balloon: Implement support for page poison tracking feature
On 22.04.20 20:21, Alexander Duyck wrote:
> From: Alexander Duyck
>
> We need to make certain to advertise support for page poison tracking if
> we want to actually get data on if the guest will be poisoning pages.
>
> Add a value for tracking the poison value being used if page poisoning is
> enabled. With this we can determine if we will need to skip page reporting
> when it is enabled in the future.
Maybe add something about the semantics
"VIRTIO_BALLOON_F_PAGE_POISON will not change the behavior of free page
hinting or ordinary balloon inflation/deflation."
I do wonder if we should just unconditionally enable
VIRTIO_BALLOON_F_PAGE_POISON here, gluing it to the QEMU compat machine
(via a property that is default-enabled, and disabled from compat machines).
Because, as Michael said, knowing that the guest is using page poisoning
might be interesting even if free page reporting is not around.
>
> Signed-off-by: Alexander Duyck
> ---
> hw/virtio/virtio-balloon.c |7 +++
> include/hw/virtio/virtio-balloon.h |1 +
> 2 files changed, 8 insertions(+)
>
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index a1d6fb52c876..5effc8b4653b 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -634,6 +634,7 @@ static void virtio_balloon_get_config(VirtIODevice *vdev,
> uint8_t *config_data)
>
> config.num_pages = cpu_to_le32(dev->num_pages);
> config.actual = cpu_to_le32(dev->actual);
> +config.poison_val = cpu_to_le32(dev->poison_val);
>
> if (dev->free_page_hint_status == FREE_PAGE_HINT_S_REQUESTED) {
> config.free_page_hint_cmd_id =
> @@ -697,6 +698,10 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
> qapi_event_send_balloon_change(vm_ram_size -
> ((ram_addr_t) dev->actual <<
> VIRTIO_BALLOON_PFN_SHIFT));
> }
> +dev->poison_val = 0;
> +if (virtio_vdev_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_POISON)) {
> +dev->poison_val = le32_to_cpu(config.poison_val);
> +}
> trace_virtio_balloon_set_config(dev->actual, oldactual);
> }
>
> @@ -854,6 +859,8 @@ static void virtio_balloon_device_reset(VirtIODevice
> *vdev)
> g_free(s->stats_vq_elem);
> s->stats_vq_elem = NULL;
> }
> +
> +s->poison_val = 0;
> }
>
> static void virtio_balloon_set_status(VirtIODevice *vdev, uint8_t status)
> diff --git a/include/hw/virtio/virtio-balloon.h
> b/include/hw/virtio/virtio-balloon.h
> index 108cff97e71a..3ca2a78e1aca 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -70,6 +70,7 @@ typedef struct VirtIOBalloon {
> uint32_t host_features;
>
> bool qemu_4_0_config_size;
> +uint32_t poison_val;
> } VirtIOBalloon;
>
> #endif
>
You still have to migrate poison_val if I am not wrong, otherwise you
would lose it during migration if I am not mistaking.
--
Thanks,
David / dhildenb
Re: [PATCH] accel/tcg: Add stub for probe_access()
On 23/04/20 10:04, David Hildenbrand wrote: >> I can only recommend you to read the thread after this previous patch, >> as I don't have the knowledge to explain...: >> https://www.mail-archive.com/qemu-devel@nongnu.org/msg689115.html > Yeah, me neither. Sounds wrong to me to have TCG-only code stick around > in !CONFIG_TCG builds. But I am pretty sure ARM people know what they > are doing. It's better if helpers are left out via obj-$(CONFIG_TCG), but it's not the end of the world if they aren't---as long as it compiles of course! This case is definitely borderline. Paolo
Re: [PATCH 01/11] MAINTAINERS: Fix KVM path expansion glob
Philippe Mathieu-Daudé wrote:
> The KVM files has been moved from target-ARCH to the target/ARCH/
> folder in commit fcf5ef2a. Fix the pathname expansion.
>
> Fixes: fcf5ef2a ("Move target-* CPU file into a target/ folder")
> Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Juan Quintela
Re: [PATCH v2 14/14] qga: Fix qmp_guest_suspend_{disk, ram}() error handling
Philippe Mathieu-Daudé writes:
> On 4/22/20 5:17 PM, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé writes:
>>
>>> On 4/22/20 3:07 PM, Markus Armbruster wrote:
The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL. Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
qmp_guest_suspend_disk() and qmp_guest_suspend_ram() pass @local_err
first to check_suspend_mode(), then to acquire_privilege(), then to
execute_async(). Continuing after errors here can only end in tears.
For instance, we risk tripping error_setv()'s assertion.
Fixes: aa59637ea1c6a4c83430933f9c44c43e6c3f1b69
Fixes: f54603b6aa765514b2519e74114a2f417759d727
Cc: Michael Roth
Signed-off-by: Markus Armbruster
---
qga/commands-win32.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 9717a8d52d..5ba56327dd 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1322,9 +1322,16 @@ void qmp_guest_suspend_disk(Error **errp)
*mode = GUEST_SUSPEND_MODE_DISK;
check_suspend_mode(*mode, &local_err);
+if (local_err) {
+goto out;
+}
acquire_privilege(SE_SHUTDOWN_NAME, &local_err);
+if (local_err) {
+goto out;
+}
execute_async(do_suspend, mode, &local_err);
+out:
if (local_err) {
>>>
>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg695647.html is
>>> slightly different by removing the if() check.
>>
>> It frees @mode unconditionally (marked --> below) I believe that's
>> wrong. execute_async() runs do_suspend() in a new thread, and passes it
>> @mode. do_suspend() frees it.
>
> Oops I missed that, good catch!
>
> Reviewed-by: Philippe Mathieu-Daudé
Thanks!
I wasn't aware of (or totally forgot about) your patch, or else I'd have
fixed it instead of redoing it. My apologies!
Re: [PATCH v3 10/19] target/arm: Restrict ARMv4 cpus to TCG accel
On 3/16/20 5:06 PM, Philippe Mathieu-Daudé wrote: KVM requires a cpu based on (at least) the ARMv7 architecture. Only enable the following ARMv4 CPUs when TCG is available: - StrongARM (SA1100/1110) - OMAP1510 (TI925T) I missed to explain, the point of this Kconfig granularity is on a KVM only build, the TCG-only CPUs can't be default-selected, so most of their devices are not pulled in. Instead at the end the KVM-only binary only contains the devices required to run the Cortex-A machines. diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index 8b89d8c4c0..0652396296 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -17,8 +17,6 @@ CONFIG_INTEGRATOR=y CONFIG_FSL_IMX31=y CONFIG_MUSICPAL=y CONFIG_MUSCA=y -CONFIG_CHEETAH=y -CONFIG_SX1=y CONFIG_NSERIES=y CONFIG_STELLARIS=y CONFIG_REALVIEW=y [...] diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig index e3d7e7694a..7fc0cff776 100644 --- a/hw/arm/Kconfig +++ b/hw/arm/Kconfig @@ -28,6 +28,7 @@ config ARM_VIRT config CHEETAH bool +select ARM_V4 select OMAP select TSC210X @@ -242,6 +243,7 @@ config COLLIE config SX1 bool +select ARM_V4 select OMAP config VERSATILE diff --git a/target/arm/Kconfig b/target/arm/Kconfig index e68c71a6ff..0d496d318a 100644 --- a/target/arm/Kconfig +++ b/target/arm/Kconfig @@ -1,2 +1,6 @@ +config ARM_V4 +depends on TCG +bool + config ARM_V7M bool
Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up
On 2020/4/23 下午3:31, Zhang, Chen wrote: Hi Jason, Please review this series when you free. Thanks Zhang Chen Sure. I wonder maybe it's better e.g you can review and collect the patches that looks good and send them to me periodically? Thanks
RE: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up
> -Original Message- > From: Jason Wang > Sent: Thursday, April 23, 2020 4:54 PM > To: Zhang, Chen ; qemu-dev de...@nongnu.org> > Cc: Zhang Chen > Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to > users and clean up > > > On 2020/4/23 下午3:31, Zhang, Chen wrote: > > Hi Jason, > > > > Please review this series when you free. > > > > Thanks > > Zhang Chen > > > > Sure. > > I wonder maybe it's better e.g you can review and collect the patches that > looks good and send them to me periodically? OK, I will queue COLO related patch as one series to you. Do I need send a pull request? or just a big patch set? Thanks Zhang Chen > > Thanks
Re: [PATCH v2] qemu-sockets: add abstract UNIX domain socket support
Adding Eric & Markus for QAPI modelling questions
On Thu, Apr 23, 2020 at 11:56:40AM +0800, xiaoqiang zhao wrote:
> unix_connect_saddr now support abstract address type
>
> By default qemu does not support abstract UNIX domain
> socket address. Add this ability to make qemu handy
> when abstract address is needed.
Was that a specific app you're using with QEMU that needs this ?
> Abstract address is marked by prefixing the address name with a '@'.
For full support of the abstract namespace we would ned to allow
the "sun_path" to contain an arbitrary mix of NULs and non-NULs
characters, and allow connect() @addrlen to be an arbitrary size.
This patch only allows a single initial NUL, and reqiures @addrlen
to be the full size of sun_path, padding with trailing NULs. This
limitation is impossible to lift with QEMU's current approach to
UNIX sockets, as it relies on passing around a NULL terminated
string, so there's no way to have embedded NULs. Since there's
no explicit length, we have to chooose between forcing the full
sun_path size as @addrlen, or forcing the string length as the
@addrlen value.
IIUC, socat makes the latter decision by default, but has a
flag to switch to the former.
[man socat]
unix-tightsocklen=[0|1]
On socket operations, pass a socket address length that does not
include the whole struct sockaddr_un record but (besides other compo‐
nents) only the relevant part of the filename or abstract string.
Default is 1.
[/man]
This actually is supported for both abstract and non-abstract
sockets, though IIUC this doesn't make a semantic difference
for non-abstract sockets.
The point is we have four possible combinations
NON-ABSTRACT + FULL SIZE
NON-ABSTRACT + MINIMAL SIZE (default)
ABSTRACT + FULL SIZE
ABSTRACT + MINIMAL SIZE (default)
With your patch doing the latter, it means QEMU supports
only two combinations
NON+ABSTRACT + FULL SIZE
ABSTRACT + MINIMAL SIZE
and also can't use "@somerealpath" for a non-abstract
socket, though admittedly this is unlikely.
Socat uses a special option to request use of abstract
sockets. eg ABSTRACT:somepath, and automatically adds
the leading NUL, so there's no need for a special "@"
character. This means that UNIX:@somepath still resolves
to a filesystem path and not a abstract socket path.
Finally, the patch as only added support for connect()
not listen(). I think if QEMU wants to support abstract
sockets we must do both, and also have unit tests added
to tests/test-util-sockets.c
The question is whether we're ok with this simple
approach in QEMU, or should do a full approach with
more explicit modelling.
ie should we change QAPI thus:
{ 'struct': 'UnixSocketAddress',
'data': {
'path': 'str',
'tight': 'bool',
'abstract': 'bool' } }
where 'tight' is a flag indicating whether to set @addrlen
to the minimal string length, or the maximum sun_path length.
And 'abstract' indicates that we automagically add a leading
NUL.
This would *not* allow for NULs in the middle of path,
but I'm not so bothered about that, since I can't see that
being widely used. If we really did need that it could be
added via a 'base64': 'bool' flag, to indicate that @path
is base64 encoded and thus may contain NULs
>From a CLI POV, this could be mapped to QAPI thus
* -chardev unix:somepath
@path==somepath
@tight==false
@abstract==false
* -chardev unix:somepath,tight
@path==somepath
@tight==true
@abstract==false
* -chardev unix-abstract:somepath
@path==somepath
@tight==false
@abstract==true
* -chardev unix-abstract:somepath,tight
@path==somepath
@tight==true
@abstract==true
>
> Signed-off-by: xiaoqiang zhao
> ---
> util/qemu-sockets.c | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index bcc06d0e01..7ba9c497ab 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -939,6 +939,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr,
> Error **errp)
> struct sockaddr_un un;
> int sock, rc;
> size_t pathlen;
> +socklen_t serverlen;
>
> if (saddr->path == NULL) {
> error_setg(errp, "unix connect: no path specified");
> @@ -963,10 +964,17 @@ static int unix_connect_saddr(UnixSocketAddress *saddr,
> Error **errp)
> un.sun_family = AF_UNIX;
> memcpy(un.sun_path, saddr->path, pathlen);
>
> +if (saddr->path[0] == '@') {
> +un.sun_path[0] = '\0';
> +serverlen = pathlen + offsetof(struct sockaddr_un, sun_path);
> +} else {
> +serverlen = sizeof(un);
> +}
> +
> /* connect to peer */
> do {
> rc = 0;
> -if (connect(sock, (struct sockaddr *) &un, sizeof(un)) < 0) {
> +if (connect(sock, (struct sockaddr *) &un, serverlen) < 0) {
> rc = -errno;
> }
> } while (rc == -EINTR);
> --
> 2.17.1
>
>
Regard
Re: [PATCH v2 09/36] tcg: Consolidate 3 bits into enum TCGTempKind
On 4/22/20 9:58 PM, Aleksandar Markovic wrote:
> сре, 22. апр 2020. у 03:27 Richard Henderson
> је написао/ла:
>>
>> The temp_fixed, temp_global, temp_local bits are all related.
>> Combine them into a single enumeration.
>>
>> Signed-off-by: Richard Henderson
>> ---
>> include/tcg/tcg.h | 20 +---
>> tcg/optimize.c| 8 +--
>> tcg/tcg.c | 122 --
>> 3 files changed, 90 insertions(+), 60 deletions(-)
>>
>> diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
>> index c48bd76b0a..3534dce77f 100644
>> --- a/include/tcg/tcg.h
>> +++ b/include/tcg/tcg.h
>> @@ -480,23 +480,27 @@ typedef enum TCGTempVal {
>> TEMP_VAL_CONST,
>> } TCGTempVal;
>>
>> +typedef enum TCGTempKind {
>> +/* Temp is dead at the end of all basic blocks. */
>> +TEMP_NORMAL,
>> +/* Temp is saved across basic blocks but dead at the end of TBs. */
>> +TEMP_LOCAL,
>> +/* Temp is saved across both basic blocks and translation blocks. */
>> +TEMP_GLOBAL,
>> +/* Temp is in a fixed register. */
>> +TEMP_FIXED,
4 cases, so currently 2 bits are enough.
>> +} TCGTempKind;
>> +
>> typedef struct TCGTemp {
>> TCGReg reg:8;
>> TCGTempVal val_type:8;
>> TCGType base_type:8;
>> TCGType type:8;
>> -unsigned int fixed_reg:1;
>> +TCGTempKind kind:3;
But in case you plan to support more cases...
>> unsigned int indirect_reg:1;
>> unsigned int indirect_base:1;
>> unsigned int mem_coherent:1;
>> unsigned int mem_allocated:1;
>> -/* If true, the temp is saved across both basic blocks and
>> - translation blocks. */
>> -unsigned int temp_global:1;
>> -/* If true, the temp is saved across basic blocks but dead
>> - at the end of translation blocks. If false, the temp is
>> - dead at the end of basic blocks. */
>> -unsigned int temp_local:1;
>> unsigned int temp_allocated:1;
>>
>> tcg_target_long val;
>> diff --git a/tcg/optimize.c b/tcg/optimize.c
>> index 53aa8e5329..afb4a9a5a9 100644
>> --- a/tcg/optimize.c
>> +++ b/tcg/optimize.c
>> @@ -116,21 +116,21 @@ static TCGTemp *find_better_copy(TCGContext *s,
>> TCGTemp *ts)
>> TCGTemp *i;
>>
>> /* If this is already a global, we can't do better. */
>> -if (ts->temp_global) {
>> +if (ts->kind >= TEMP_GLOBAL) {
>> return ts;
>> }
>>
>> /* Search for a global first. */
>> for (i = ts_info(ts)->next_copy; i != ts; i = ts_info(i)->next_copy) {
>> -if (i->temp_global) {
>> +if (i->kind >= TEMP_GLOBAL) {
>> return i;
>> }
>> }
>>
>> /* If it is a temp, search for a temp local. */
>> -if (!ts->temp_local) {
>> +if (ts->kind == TEMP_NORMAL) {
>> for (i = ts_info(ts)->next_copy; i != ts; i =
>> ts_info(i)->next_copy) {
>> -if (ts->temp_local) {
>> +if (i->kind >= TEMP_LOCAL) {
>> return i;
>> }
>> }
>> diff --git a/tcg/tcg.c b/tcg/tcg.c
>> index dd4b3d7684..eaf81397a3 100644
>> --- a/tcg/tcg.c
>> +++ b/tcg/tcg.c
>> @@ -1155,7 +1155,7 @@ static inline TCGTemp *tcg_global_alloc(TCGContext *s)
>> tcg_debug_assert(s->nb_globals == s->nb_temps);
>> s->nb_globals++;
>> ts = tcg_temp_alloc(s);
>> -ts->temp_global = 1;
>> +ts->kind = TEMP_GLOBAL;
>>
>> return ts;
>> }
>> @@ -1172,7 +1172,7 @@ static TCGTemp *tcg_global_reg_new_internal(TCGContext
>> *s, TCGType type,
>> ts = tcg_global_alloc(s);
>> ts->base_type = type;
>> ts->type = type;
>> -ts->fixed_reg = 1;
>> +ts->kind = TEMP_FIXED;
>> ts->reg = reg;
>> ts->name = name;
>> tcg_regset_set_reg(s->reserved_regs, reg);
>> @@ -1199,7 +1199,7 @@ TCGTemp *tcg_global_mem_new_internal(TCGType type,
>> TCGv_ptr base,
>> bigendian = 1;
>> #endif
>>
>> -if (!base_ts->fixed_reg) {
>> +if (base_ts->kind != TEMP_FIXED) {
>> /* We do not support double-indirect registers. */
>> tcg_debug_assert(!base_ts->indirect_reg);
>> base_ts->indirect_base = 1;
>> @@ -1247,6 +1247,7 @@ TCGTemp *tcg_global_mem_new_internal(TCGType type,
>> TCGv_ptr base,
>> TCGTemp *tcg_temp_new_internal(TCGType type, bool temp_local)
>> {
>> TCGContext *s = tcg_ctx;
>> +TCGTempKind kind = temp_local ? TEMP_LOCAL : TEMP_NORMAL;
>> TCGTemp *ts;
>> int idx, k;
>>
>> @@ -1259,7 +1260,7 @@ TCGTemp *tcg_temp_new_internal(TCGType type, bool
>> temp_local)
>> ts = &s->temps[idx];
>> ts->temp_allocated = 1;
>> tcg_debug_assert(ts->base_type == type);
>> -tcg_debug_assert(ts->temp_local == temp_local);
>> +tcg_debug_assert(ts->kind == kind);
>> } else {
>> ts = tcg_temp_alloc(s);
>> if (TCG_TARGET_REG_BITS == 32 && type == TCG_TYPE_I64) {
>> @@ -1268,18 +1269,18 @@ TCGTemp *tcg_temp_new_internal(TCGType type, bool
>> temp_local)
>> ts->base_type = type;
>>
[PATCH QEMU v2 4/5] ARM: PL061: Add gpiodev support
Make the PL061 GPIO controller user-creatable, and allow the user to tie
a newly created instance to a gpiochip on the host.
To create a new GPIO controller, the QEMU command line must be augmented
with:
-device pl061,host=
with the name or label of the gpiochip on the host.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
hw/gpio/pl061.c | 35 +++
qemu-options.hx | 9 +
2 files changed, 44 insertions(+)
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index 74ba733a8a5e8ca5..98204f9a586ae8c8 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -12,11 +12,14 @@
#include "hw/arm/fdt.h"
#include "hw/gpio/pl061.h"
#include "hw/irq.h"
+#include "hw/qdev-properties.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
+#include "qapi/error.h"
#include "qemu/log.h"
#include "qemu/module.h"
#include "sysemu/device_tree.h"
+#include "sysemu/gpiodev.h"
//#define DEBUG_PL061 1
@@ -41,6 +44,9 @@ static const uint8_t pl061_id_luminary[12] =
typedef struct PL061State {
SysBusDevice parent_obj;
+#ifdef CONFIG_GPIODEV
+char *host;
+#endif
MemoryRegion iomem;
uint32_t locked;
uint32_t data;
@@ -370,10 +376,39 @@ static void pl061_init(Object *obj)
qdev_init_gpio_out(dev, s->out, 8);
}
+#ifdef CONFIG_GPIODEV
+static Property pl061_properties[] = {
+DEFINE_PROP_STRING("host", PL061State, host),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pl061_realize(DeviceState *dev, Error **errp)
+{
+PL061State *s = PL061(dev);
+
+if (!dev->opts) {
+/* Not created by user */
+return;
+}
+
+if (!s->host) {
+error_setg(errp, "'host' property is required");
+return;
+}
+
+qemu_gpiodev_add(dev, s->host, 8, errp);
+}
+#endif /* CONFIG_GPIODEV */
+
static void pl061_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+#ifdef CONFIG_GPIODEV
+device_class_set_props(dc, pl061_properties);
+dc->realize = pl061_realize;
+dc->user_creatable = true;
+#endif
dc->vmsd = &vmstate_pl061;
dc->reset = &pl061_reset;
}
diff --git a/qemu-options.hx b/qemu-options.hx
index 292d4e7c0cef6097..182de7fb63923b38 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -875,6 +875,15 @@ SRST
``-device isa-ipmi-bt,bmc=id[,ioport=val][,irq=val]``
Like the KCS interface, but defines a BT interface. The default port
is 0xe4 and the default interrupt is 5.
+
+#ifdef CONFIG_GPIODEV
+``-device pl061,host=gpiochip``
+Add a PL061 GPIO controller, and map its virtual GPIO lines to a GPIO
+controller on the host.
+
+``host=gpiochip``
+The name or label of the GPIO controller on the host.
+#endif
ERST
DEF("name", HAS_ARG, QEMU_OPTION_name,
--
2.17.1
[PATCH QEMU v2 3/5] Add a GPIO backend using libgpiod
Add a GPIO controller backend, to connect virtual GPIOs on the guest to
physical GPIOs on the host. This allows the guest to control any
external device connected to the physical GPIOs.
Features and limitations:
- The backend uses libgpiod on Linux,
- For now only GPIO outputs are supported,
- The number of GPIO lines mapped is limited to the number of GPIO
lines available on the virtual GPIO controller.
Future work:
- GPIO inputs,
- GPIO line configuration,
- Optimizations for controlling multiple GPIO lines at once,
- ...
Signed-off-by: Geert Uytterhoeven
---
v2:
- Drop vgpios and gpios parameters, and map the full gpiochip instead,
- Replace hardcoded PL061 instance by multiple dynamic instances,
registered through qemu_gpiodev_add().
---
MAINTAINERS | 6 +++
backends/Makefile.objs | 2 +
backends/gpiodev.c | 94
configure| 28
include/sysemu/gpiodev.h | 12 +
5 files changed, 142 insertions(+)
create mode 100644 backends/gpiodev.c
create mode 100644 include/sysemu/gpiodev.h
diff --git a/MAINTAINERS b/MAINTAINERS
index e760f65270d29d5d..a70af47430083d14 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -607,6 +607,12 @@ F: include/hw/arm/digic.h
F: hw/*/digic*
F: include/hw/*/digic*
+GPIO device backend
+M: Geert Uytterhoeven
+S: Supported
+F: backends/gpiodev.c
+F: include/sysemu/gpiodev.h
+
Goldfish RTC
M: Anup Patel
M: Alistair Francis
diff --git a/backends/Makefile.objs b/backends/Makefile.objs
index 28a847cd571d96ed..ee658e797454119a 100644
--- a/backends/Makefile.objs
+++ b/backends/Makefile.objs
@@ -21,3 +21,5 @@ common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
common-obj-$(CONFIG_GIO) += dbus-vmstate.o
dbus-vmstate.o-cflags = $(GIO_CFLAGS)
dbus-vmstate.o-libs = $(GIO_LIBS)
+
+common-obj-$(CONFIG_GPIODEV) += gpiodev.o
diff --git a/backends/gpiodev.c b/backends/gpiodev.c
new file mode 100644
index ..df1bd0113c7b2985
--- /dev/null
+++ b/backends/gpiodev.c
@@ -0,0 +1,94 @@
+/*
+ * QEMU GPIO Backend
+ *
+ * Copyright (C) 2018-2020 Glider bv
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include
+#include
+
+#include "qemu/osdep.h"
+#include "qemu/config-file.h"
+#include "qemu/cutils.h"
+#include "qemu/error-report.h"
+#include "qemu/module.h"
+#include "qemu/option.h"
+#include "qapi/error.h"
+
+#include "sysemu/gpiodev.h"
+
+#include "hw/irq.h"
+#include "hw/qdev-core.h"
+
+static void gpiodev_irq_handler(void *opaque, int n, int level)
+{
+struct gpiod_line *line = opaque;
+int status;
+
+status = gpiod_line_set_value(line, level);
+if (status < 0) {
+struct gpiod_chip *chip = gpiod_line_get_chip(line);
+
+error_report("%s/%s: Cannot set GPIO line %u: %s",
+ gpiod_chip_name(chip), gpiod_chip_label(chip),
+ gpiod_line_offset(line), strerror(errno));
+}
+}
+
+static int gpiodev_map_line(DeviceState *dev, struct gpiod_chip *chip,
+unsigned int gpio, Error **errp)
+{
+struct gpiod_line *line;
+qemu_irq irq;
+int status;
+
+line = gpiod_chip_get_line(chip, gpio);
+if (!line) {
+error_setg(errp, "Cannot obtain GPIO line %u: %s", gpio,
+ strerror(errno));
+return -1;
+}
+
+status = gpiod_line_request_output(line, "qemu", 0);
+if (status < 0) {
+error_setg(errp, "Cannot request GPIO line %u for output: %s", gpio,
+ strerror(errno));
+return status;
+}
+
+irq = qemu_allocate_irq(gpiodev_irq_handler, line, 0);
+qdev_connect_gpio_out(dev, gpio, irq);
+return 0;
+}
+
+void qemu_gpiodev_add(DeviceState *dev, const char *name, unsigned int maxgpio,
+ Error **errp)
+{
+struct gpiod_chip *chip;
+unsigned int i, n;
+int status;
+
+chip = gpiod_chip_open_lookup(name);
+if (!chip) {
+error_setg(errp, "Cannot open GPIO chip %s: %s", name,
+ strerror(errno));
+return;
+}
+
+n = gpiod_chip_num_lines(chip);
+if (n > maxgpio) {
+warn_report("Last %u GPIO line(s) will not be mapped", n - maxgpio);
+n = maxgpio;
+}
+
+for (i = 0; i < n; i++) {
+status = gpiodev_map_line(dev, chip, i, errp);
+if (status < 0) {
+return;
+}
+}
+
+info_report("Mapped %u GPIO lines", n);
+}
diff --git a/configure b/configure
index 23b5e93752b6a259..8b133402ef727c8e 100755
--- a/configure
+++ b/configure
@@ -509,6 +509,7 @@ libpmem=""
default_devices="yes"
plugins="no"
fuzzing="no"
+gpio=""
supported_cpu="no"
supported_os="no"
@@ -1601,6 +1602,10 @@ for opt do
;;
--gdb=*) gdb_bin="$optarg"
;;
+ --disable-gpio) gpio="no"
+ ;;
+ --enable-gpio) gpio="yes"
+ ;;
*)
echo "ERROR: unknown option $opt"
echo "Try '$0 --help' for more information"
@@ -1894,6 +1899,7 @@ disabled with --di
[PATCH QEMU v2 0/5] Add a GPIO backend
Hi all,
This patch series adds a GPIO controller backend, to connect virtual
GPIOs on the guest to physical GPIOs on the host, and enables support
for this using user-creatable PL061 GPIO controller instances. This
allows the guest to control any external device connected to the
physical GPIOs.
While this can be used with an upstream Linux kernel (e.g. using a
dedicated GPIO controller connected to an external bus), proper
isolation and assignment of GPIOs to virtual machines depends on the
GPIO Aggregator[1], which has not been accepted in Linux upstream yet.
Aggregating GPIOs and exposing them as a new gpiochip was suggested in
response to my proof-of-concept for GPIO virtualization with QEMU[2][3].
Features and limitations:
- The backend uses libgpiod on Linux,
- For now only GPIO outputs are supported,
- The number of GPIO lines mapped is limited to the number of GPIO
lines available on the virtual GPIO controller (i.e. 8 on PL061).
Future work:
- GPIO inputs,
- GPIO line configuration,
- Optimizations for controlling multiple GPIO lines at once,
- ...
This series contains 5 patches:
- The first two patches refactor the existing code for reuse,
- The third patch adds a gneric GPIO backend using libgpiod,
- The fourth patch adds gpiodev support to the PL061 driver,
- The fifth patch adds dynamic PL061 support to ARM virt.
Changes compared to v1[2]:
- Drop vgpios and gpios parameters, and map the full gpiochip instead,
- Replace the single hardcoded PL061 instance (created by ARM virt) by
multiple dynamically created instances, one per imported GPIO
controller.
For testing, I have pushed this series to the topic/gpio-backend-v2
branch of my git repository at https://github.com/geertu/qemu.git.
Sample session on the Renesas Salvator-XS development board:
- Unbind keyboard (shared with LEDs) from gpio-keys driver:
host$ echo keys > /sys/bus/platform/drivers/gpio-keys/unbind
- Aggregate GPIO lines connected to LEDs into a new virtual GPIO chip:
host$ echo e6055400.gpio 11-13 \
> /sys/bus/platform/drivers/gpio-aggregator/new_device
gpio-aggregator gpio-aggregator.0: 0 => gpio-371
gpio-aggregator gpio-aggregator.0: 1 => gpio-372
gpio-aggregator gpio-aggregator.0: 2 => gpio-373
gpiochip_find_base: found new base at 343
gpio gpiochip10: (gpio-aggregator.0): added GPIO chardev (254:10)
gpiochip_setup_dev: registered GPIOs 343 to 345 on device: gpiochip10
(gpio-aggregator.0)
- Adjust permissions on /dev/gpiochip10 (optional)
- Launch QEMU:
host$ aarch64-softmmu/qemu-system-aarch64 -enable-kvm -M virt \
-cpu cortex-a57 -m 1024 -nographic -kernel /path/to/Image \
-device pl061,host=gpio-aggregator.0
...
pl061_gpio c00.gpio: PL061 GPIO chip registered
...
- Control LEDs:
guest$ gpioset gpiochip0 0=0 1=1 # LED4 OFF, LED5 ON
guest$ gpioset gpiochip0 0=1 1=0 # LED4 ON, LED5 OFF
Thanks for your comments!
[1] "[PATCH v6 0/8] gpio: Add GPIO Aggregator"
(https://lore.kernel.org/linux-gpio/20200324135328.5796-1-geert+rene...@glider.be/)
[2] "[PATCH QEMU POC] Add a GPIO backend"
(https://lore.kernel.org/linux-gpio/20181003152521.23144-1-geert+rene...@glider.be/)
[3] "Getting To Blinky: Virt Edition / Making device pass-through work
on embedded ARM"
(https://fosdem.org/2019/schedule/event/vai_getting_to_blinky/)
Geert Uytterhoeven (5):
ARM: PL061: Move TYPE_PL061 to hw/gpio/pl061.h
ARM: PL061: Extract pl061_create_fdt()
Add a GPIO backend using libgpiod
ARM: PL061: Add gpiodev support
hw/arm/virt: Add dynamic PL061 GPIO support
MAINTAINERS | 7 +++
backends/Makefile.objs | 2 +
backends/gpiodev.c | 94
configure| 28
hw/arm/sysbus-fdt.c | 18
hw/arm/virt.c| 21 ++---
hw/gpio/pl061.c | 79 -
include/hw/gpio/pl061.h | 23 ++
include/sysemu/gpiodev.h | 12 +
qemu-options.hx | 9
10 files changed, 275 insertions(+), 18 deletions(-)
create mode 100644 backends/gpiodev.c
create mode 100644 include/hw/gpio/pl061.h
create mode 100644 include/sysemu/gpiodev.h
--
2.17.1
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH QEMU v2 1/5] ARM: PL061: Move TYPE_PL061 to hw/gpio/pl061.h
Move the definition of TYPE_PL061 to a new header file, so it can be
used outside the driver.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
MAINTAINERS | 1 +
hw/gpio/pl061.c | 2 +-
include/hw/gpio/pl061.h | 16
3 files changed, 18 insertions(+), 1 deletion(-)
create mode 100644 include/hw/gpio/pl061.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 8cbc1fac2bfcec86..e760f65270d29d5d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -538,6 +538,7 @@ F: hw/dma/pl080.c
F: include/hw/dma/pl080.h
F: hw/dma/pl330.c
F: hw/gpio/pl061.c
+F: include/hw/gpio/pl061.h
F: hw/input/pl050.c
F: hw/intc/pl190.c
F: hw/sd/pl181.c
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index 2a828260bdb0b946..e776c09e474216ef 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -9,6 +9,7 @@
*/
#include "qemu/osdep.h"
+#include "hw/gpio/pl061.h"
#include "hw/irq.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
@@ -33,7 +34,6 @@ static const uint8_t pl061_id[12] =
static const uint8_t pl061_id_luminary[12] =
{ 0x00, 0x00, 0x00, 0x00, 0x61, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1 };
-#define TYPE_PL061 "pl061"
#define PL061(obj) OBJECT_CHECK(PL061State, (obj), TYPE_PL061)
typedef struct PL061State {
diff --git a/include/hw/gpio/pl061.h b/include/hw/gpio/pl061.h
new file mode 100644
index ..78cc40c52679dc4e
--- /dev/null
+++ b/include/hw/gpio/pl061.h
@@ -0,0 +1,16 @@
+/*
+ * Arm PrimeCell PL061 General Purpose IO with additional Luminary Micro
+ * Stellaris bits.
+ *
+ * Copyright (c) 2007 CodeSourcery.
+ * Written by Paul Brook
+ *
+ * This code is licensed under the GPL.
+ */
+
+#ifndef PL061_GPIO_H
+#define PL061_GPIO_H
+
+#define TYPE_PL061 "pl061"
+
+#endif /* PL061_GPIO_H */
--
2.17.1
[PATCH QEMU v2 5/5] hw/arm/virt: Add dynamic PL061 GPIO support
Add support for dynamically instantiating PL061 GPIO controller
instances in ARM virt. Device tree nodes are created dynamically.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
hw/arm/sysbus-fdt.c | 18 ++
hw/arm/virt.c | 1 +
2 files changed, 19 insertions(+)
diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 6b6906f4cfc36198..493583218d176d0a 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -32,6 +32,7 @@
#include "sysemu/device_tree.h"
#include "sysemu/tpm.h"
#include "hw/platform-bus.h"
+#include "hw/gpio/pl061.h"
#include "hw/vfio/vfio-platform.h"
#include "hw/vfio/vfio-calxeda-xgmac.h"
#include "hw/vfio/vfio-amd-xgbe.h"
@@ -468,6 +469,22 @@ static int add_tpm_tis_fdt_node(SysBusDevice *sbdev, void
*opaque)
return 0;
}
+/*
+ * add_pl061_node: Create a DT node for a PL061 GPIO controller
+ */
+static int add_pl061_node(SysBusDevice *sbdev, void *opaque)
+{
+PlatformBusFDTData *data = opaque;
+PlatformBusDevice *pbus = data->pbus;
+void *fdt = data->fdt;
+
+pl061_create_fdt(fdt, data->pbus_node_name, 1,
+ platform_bus_get_mmio_addr(pbus, sbdev, 0), 0x1000,
+ platform_bus_get_irqn(pbus, sbdev, 0) + data->irq_start,
+ qemu_fdt_get_phandle(fdt, "/apb-pclk"));
+return 0;
+}
+
static int no_fdt_node(SysBusDevice *sbdev, void *opaque)
{
return 0;
@@ -489,6 +506,7 @@ static const BindingEntry bindings[] = {
VFIO_PLATFORM_BINDING("amd,xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
#endif
TYPE_BINDING(TYPE_TPM_TIS_SYSBUS, add_tpm_tis_fdt_node),
+TYPE_BINDING(TYPE_PL061, add_pl061_node),
TYPE_BINDING(TYPE_RAMFB_DEVICE, no_fdt_node),
TYPE_BINDING("", NULL), /* last element */
};
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index c88c8850fbe00bdb..191594db09422d54 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2178,6 +2178,7 @@ static void virt_machine_class_init(ObjectClass *oc, void
*data)
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_RAMFB_DEVICE);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_PLATFORM);
machine_class_allow_dynamic_sysbus_dev(mc, TYPE_TPM_TIS_SYSBUS);
+machine_class_allow_dynamic_sysbus_dev(mc, TYPE_PL061);
mc->block_default_type = IF_VIRTIO;
mc->no_cdrom = 1;
mc->pci_allow_0_address = true;
--
2.17.1
[PATCH QEMU v2 2/5] ARM: PL061: Extract pl061_create_fdt()
Move the code to create the DT node for the PL061 GPIO controller from
hw/arm/virt.c to the PL061 driver, so it can be reused.
While at it, make the created node comply with the PL061 Device Tree
bindings:
- Use generic node name "gpio" instead of "pl061",
- Add missing "#interrupt-cells" and "interrupt-controller"
properties.
Signed-off-by: Geert Uytterhoeven
---
v2:
- New.
---
hw/arm/virt.c | 20 +++-
hw/gpio/pl061.c | 42 +
include/hw/gpio/pl061.h | 7 +++
3 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 7dc96abf72cf2b9a..c88c8850fbe00bdb 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -40,6 +40,7 @@
#include "hw/arm/primecell.h"
#include "hw/arm/virt.h"
#include "hw/block/flash.h"
+#include "hw/gpio/pl061.h"
#include "hw/vfio/vfio-calxeda-xgmac.h"
#include "hw/vfio/vfio-amd-xgbe.h"
#include "hw/display/ramfb.h"
@@ -807,30 +808,16 @@ static void virt_powerdown_req(Notifier *n, void *opaque)
static void create_gpio(const VirtMachineState *vms)
{
-char *nodename;
DeviceState *pl061_dev;
hwaddr base = vms->memmap[VIRT_GPIO].base;
hwaddr size = vms->memmap[VIRT_GPIO].size;
int irq = vms->irqmap[VIRT_GPIO];
-const char compat[] = "arm,pl061\0arm,primecell";
pl061_dev = sysbus_create_simple("pl061", base,
qdev_get_gpio_in(vms->gic, irq));
-uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt);
-nodename = g_strdup_printf("/pl061@%" PRIx64, base);
-qemu_fdt_add_subnode(vms->fdt, nodename);
-qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
- 2, base, 2, size);
-qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat, sizeof(compat));
-qemu_fdt_setprop_cell(vms->fdt, nodename, "#gpio-cells", 2);
-qemu_fdt_setprop(vms->fdt, nodename, "gpio-controller", NULL, 0);
-qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
- GIC_FDT_IRQ_TYPE_SPI, irq,
- GIC_FDT_IRQ_FLAGS_LEVEL_HI);
-qemu_fdt_setprop_cell(vms->fdt, nodename, "clocks", vms->clock_phandle);
-qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk");
-qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
+uint32_t phandle = pl061_create_fdt(vms->fdt, "", 2, base, size, irq,
+vms->clock_phandle);
gpio_key_dev = sysbus_create_simple("gpio-key", -1,
qdev_get_gpio_in(pl061_dev, 3));
@@ -846,7 +833,6 @@ static void create_gpio(const VirtMachineState *vms)
KEY_POWER);
qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
"gpios", phandle, 3, 0);
-g_free(nodename);
}
static void create_virtio_devices(const VirtMachineState *vms)
diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
index e776c09e474216ef..74ba733a8a5e8ca5 100644
--- a/hw/gpio/pl061.c
+++ b/hw/gpio/pl061.c
@@ -9,12 +9,14 @@
*/
#include "qemu/osdep.h"
+#include "hw/arm/fdt.h"
#include "hw/gpio/pl061.h"
#include "hw/irq.h"
#include "hw/sysbus.h"
#include "migration/vmstate.h"
#include "qemu/log.h"
#include "qemu/module.h"
+#include "sysemu/device_tree.h"
//#define DEBUG_PL061 1
@@ -397,3 +399,43 @@ static void pl061_register_types(void)
}
type_init(pl061_register_types)
+
+/*
+ * pl061_create_fdt: Create a DT node for a PL061 GPIO controller
+ * @fdt: device tree blob
+ * @parent: name of the parent node
+ * @n_cells: value of #address-cells and #size-cells
+ * @base: base address of the controller's register block
+ * @size: size of the controller's register block
+ * @irq: interrupt number
+ * @clock: phandle of the apb-pclk clock
+ *
+ * Return value: a phandle referring to the created DT node.
+ *
+ * See the DT Binding Documentation in the Linux kernel source tree:
+ * Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
+ */
+uint32_t pl061_create_fdt(void *fdt, const char *parent, unsigned int n_cells,
+ hwaddr base, hwaddr size, int irq, uint32_t clock)
+{
+char *nodename = g_strdup_printf("%s/gpio@%" PRIx64, parent, base);
+static const char compat[] = "arm,pl061\0arm,primecell";
+uint32_t phandle = qemu_fdt_alloc_phandle(fdt);
+
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", n_cells, base, n_cells,
+ size);
+qemu_fdt_setprop(fdt, nodename, "compatible", compat, sizeof(compat));
+qemu_fdt_setprop_cell(fdt, nodename, "#gpio-cells", 2);
+qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
+qemu_fdt_setprop_cell(fdt, nodename, "#interrupt-cells", 2);
+qemu_fdt_setprop(fdt, nodename, "interrupt-controller", NULL, 0);
+qemu_fdt_setprop_cells(fdt, nodename, "interrupts", GIC_FDT_IRQ_TYPE_S
Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up
On 2020/4/23 下午4:59, Zhang, Chen wrote: -Original Message- From: Jason Wang Sent: Thursday, April 23, 2020 4:54 PM To: Zhang, Chen ; qemu-dev Cc: Zhang Chen Subject: Re: [PATCH 0/2] net/colo-compare.c: Expose "max_queue_size" to users and clean up On 2020/4/23 下午3:31, Zhang, Chen wrote: Hi Jason, Please review this series when you free. Thanks Zhang Chen Sure. I wonder maybe it's better e.g you can review and collect the patches that looks good and send them to me periodically? OK, I will queue COLO related patch as one series to you. Do I need send a pull request? or just a big patch set? I prefer big patch set. Thanks Thanks Zhang Chen Thanks
Re: [PATCH v2 25/36] tcg: Remove tcg_gen_dup{8,16,32,64}i_vec
Richard Henderson writes:
> These interfaces have been replaced by tcg_gen_dupi_vec
> and tcg_constant_vec.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
> ---
> include/tcg/tcg-op.h | 4
> tcg/tcg-op-vec.c | 20
> 2 files changed, 24 deletions(-)
>
> diff --git a/include/tcg/tcg-op.h b/include/tcg/tcg-op.h
> index 11ed9192f7..a39eb13ff0 100644
> --- a/include/tcg/tcg-op.h
> +++ b/include/tcg/tcg-op.h
> @@ -959,10 +959,6 @@ void tcg_gen_mov_vec(TCGv_vec, TCGv_vec);
> void tcg_gen_dup_i32_vec(unsigned vece, TCGv_vec, TCGv_i32);
> void tcg_gen_dup_i64_vec(unsigned vece, TCGv_vec, TCGv_i64);
> void tcg_gen_dup_mem_vec(unsigned vece, TCGv_vec, TCGv_ptr, tcg_target_long);
> -void tcg_gen_dup8i_vec(TCGv_vec, uint32_t);
> -void tcg_gen_dup16i_vec(TCGv_vec, uint32_t);
> -void tcg_gen_dup32i_vec(TCGv_vec, uint32_t);
> -void tcg_gen_dup64i_vec(TCGv_vec, uint64_t);
> void tcg_gen_dupi_vec(unsigned vece, TCGv_vec, uint64_t);
> void tcg_gen_add_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec b);
> void tcg_gen_sub_vec(unsigned vece, TCGv_vec r, TCGv_vec a, TCGv_vec b);
> diff --git a/tcg/tcg-op-vec.c b/tcg/tcg-op-vec.c
> index 6343046e18..a9c16d85c5 100644
> --- a/tcg/tcg-op-vec.c
> +++ b/tcg/tcg-op-vec.c
> @@ -284,26 +284,6 @@ void tcg_gen_dupi_vec(unsigned vece, TCGv_vec dest,
> uint64_t val)
> tcg_gen_mov_vec(dest, tcg_constant_vec(type, vece, val));
> }
>
> -void tcg_gen_dup64i_vec(TCGv_vec dest, uint64_t val)
> -{
> -tcg_gen_dupi_vec(MO_64, dest, val);
> -}
> -
> -void tcg_gen_dup32i_vec(TCGv_vec dest, uint32_t val)
> -{
> -tcg_gen_dupi_vec(MO_32, dest, val);
> -}
> -
> -void tcg_gen_dup16i_vec(TCGv_vec dest, uint32_t val)
> -{
> -tcg_gen_dupi_vec(MO_16, dest, val);
> -}
> -
> -void tcg_gen_dup8i_vec(TCGv_vec dest, uint32_t val)
> -{
> -tcg_gen_dupi_vec(MO_8, dest, val);
> -}
> -
> void tcg_gen_dup_i64_vec(unsigned vece, TCGv_vec r, TCGv_i64 a)
> {
> TCGArg ri = tcgv_vec_arg(r);
--
Alex Bennée
Re: [PATCH QEMU v2 0/5] Add a GPIO backend
Patchew URL: https://patchew.org/QEMU/20200423090118.11199-1-geert+rene...@glider.be/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === CC chardev/char-pipe.o CC chardev/char-pty.o Warning, treated as error: /tmp/qemu-test/src/docs/../qemu-options.hx:881:Unexpected indentation. CC chardev/char-ringbuf.o CC chardev/char-serial.o Warning, treated as error: /tmp/qemu-test/src/docs/../qemu-options.hx:881:Unexpected indentation. CC chardev/char-socket.o CC chardev/char-stdio.o --- CC blockdev-nbd.o CC iothread.o CC job-qmp.o make: *** [Makefile:1115: .docs_system_qemu.1_docs_system_qemu-block-drivers.7_docs_system_qemu-cpu-models.7.sentinel.] Error 2 make: *** Deleting file '.docs_system_qemu.1_docs_system_qemu-block-drivers.7_docs_system_qemu-cpu-models.7.sentinel.' make: *** Waiting for unfinished jobs make: *** [Makefile:1104: docs/system/index.html] Error 2 Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=583b10e192d841ea985fe9474e9cce5b', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-41xb7be_/src/docker-src.2020-04-23-05.09.56.10248:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=583b10e192d841ea985fe9474e9cce5b make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-41xb7be_/src' make: *** [docker-run-test-debug@fedora] Error 2 real3m30.232s user0m8.457s The full log is available at http://patchew.org/logs/20200423090118.11199-1-geert+rene...@glider.be/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH QEMU v2 1/5] ARM: PL061: Move TYPE_PL061 to hw/gpio/pl061.h
On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> Move the definition of TYPE_PL061 to a new header file, so it can be
> used outside the driver.
>
> Signed-off-by: Geert Uytterhoeven
> ---
> v2:
> - New.
> ---
> MAINTAINERS | 1 +
> hw/gpio/pl061.c | 2 +-
> include/hw/gpio/pl061.h | 16
> 3 files changed, 18 insertions(+), 1 deletion(-)
> create mode 100644 include/hw/gpio/pl061.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8cbc1fac2bfcec86..e760f65270d29d5d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -538,6 +538,7 @@ F: hw/dma/pl080.c
> F: include/hw/dma/pl080.h
> F: hw/dma/pl330.c
> F: hw/gpio/pl061.c
> +F: include/hw/gpio/pl061.h
> F: hw/input/pl050.c
> F: hw/intc/pl190.c
> F: hw/sd/pl181.c
> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
> index 2a828260bdb0b946..e776c09e474216ef 100644
> --- a/hw/gpio/pl061.c
> +++ b/hw/gpio/pl061.c
> @@ -9,6 +9,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "hw/gpio/pl061.h"
> #include "hw/irq.h"
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> @@ -33,7 +34,6 @@ static const uint8_t pl061_id[12] =
> static const uint8_t pl061_id_luminary[12] =
>{ 0x00, 0x00, 0x00, 0x00, 0x61, 0x00, 0x18, 0x01, 0x0d, 0xf0, 0x05, 0xb1 };
>
> -#define TYPE_PL061 "pl061"
> #define PL061(obj) OBJECT_CHECK(PL061State, (obj), TYPE_PL061)
>
> typedef struct PL061State {
> diff --git a/include/hw/gpio/pl061.h b/include/hw/gpio/pl061.h
> new file mode 100644
> index ..78cc40c52679dc4e
> --- /dev/null
> +++ b/include/hw/gpio/pl061.h
> @@ -0,0 +1,16 @@
> +/*
> + * Arm PrimeCell PL061 General Purpose IO with additional Luminary Micro
> + * Stellaris bits.
> + *
> + * Copyright (c) 2007 CodeSourcery.
> + * Written by Paul Brook
> + *
> + * This code is licensed under the GPL.
> + */
> +
> +#ifndef PL061_GPIO_H
> +#define PL061_GPIO_H
> +
> +#define TYPE_PL061 "pl061"
> +
> +#endif /* PL061_GPIO_H */
>
Reviewed-by: Philippe Mathieu-Daudé
Re: [PATCH QEMU v2 2/5] ARM: PL061: Extract pl061_create_fdt()
On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> Move the code to create the DT node for the PL061 GPIO controller from
> hw/arm/virt.c to the PL061 driver, so it can be reused.
>
> While at it, make the created node comply with the PL061 Device Tree
> bindings:
> - Use generic node name "gpio" instead of "pl061",
I'd split this patch in 2 (first one being fixing missing properties).
> - Add missing "#interrupt-cells" and "interrupt-controller"
> properties.
>
> Signed-off-by: Geert Uytterhoeven
> ---
> v2:
> - New.
> ---
> hw/arm/virt.c | 20 +++-
> hw/gpio/pl061.c | 42 +
> include/hw/gpio/pl061.h | 7 +++
> 3 files changed, 52 insertions(+), 17 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 7dc96abf72cf2b9a..c88c8850fbe00bdb 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -40,6 +40,7 @@
> #include "hw/arm/primecell.h"
> #include "hw/arm/virt.h"
> #include "hw/block/flash.h"
> +#include "hw/gpio/pl061.h"
> #include "hw/vfio/vfio-calxeda-xgmac.h"
> #include "hw/vfio/vfio-amd-xgbe.h"
> #include "hw/display/ramfb.h"
> @@ -807,30 +808,16 @@ static void virt_powerdown_req(Notifier *n, void
> *opaque)
>
> static void create_gpio(const VirtMachineState *vms)
> {
> -char *nodename;
> DeviceState *pl061_dev;
> hwaddr base = vms->memmap[VIRT_GPIO].base;
> hwaddr size = vms->memmap[VIRT_GPIO].size;
> int irq = vms->irqmap[VIRT_GPIO];
> -const char compat[] = "arm,pl061\0arm,primecell";
>
> pl061_dev = sysbus_create_simple("pl061", base,
> qdev_get_gpio_in(vms->gic, irq));
>
> -uint32_t phandle = qemu_fdt_alloc_phandle(vms->fdt);
> -nodename = g_strdup_printf("/pl061@%" PRIx64, base);
> -qemu_fdt_add_subnode(vms->fdt, nodename);
> -qemu_fdt_setprop_sized_cells(vms->fdt, nodename, "reg",
> - 2, base, 2, size);
> -qemu_fdt_setprop(vms->fdt, nodename, "compatible", compat,
> sizeof(compat));
> -qemu_fdt_setprop_cell(vms->fdt, nodename, "#gpio-cells", 2);
> -qemu_fdt_setprop(vms->fdt, nodename, "gpio-controller", NULL, 0);
> -qemu_fdt_setprop_cells(vms->fdt, nodename, "interrupts",
> - GIC_FDT_IRQ_TYPE_SPI, irq,
> - GIC_FDT_IRQ_FLAGS_LEVEL_HI);
> -qemu_fdt_setprop_cell(vms->fdt, nodename, "clocks", vms->clock_phandle);
> -qemu_fdt_setprop_string(vms->fdt, nodename, "clock-names", "apb_pclk");
> -qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", phandle);
> +uint32_t phandle = pl061_create_fdt(vms->fdt, "", 2, base, size, irq,
> +vms->clock_phandle);
>
> gpio_key_dev = sysbus_create_simple("gpio-key", -1,
> qdev_get_gpio_in(pl061_dev, 3));
> @@ -846,7 +833,6 @@ static void create_gpio(const VirtMachineState *vms)
>KEY_POWER);
> qemu_fdt_setprop_cells(vms->fdt, "/gpio-keys/poweroff",
> "gpios", phandle, 3, 0);
> -g_free(nodename);
> }
>
> static void create_virtio_devices(const VirtMachineState *vms)
> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
> index e776c09e474216ef..74ba733a8a5e8ca5 100644
> --- a/hw/gpio/pl061.c
> +++ b/hw/gpio/pl061.c
> @@ -9,12 +9,14 @@
> */
>
> #include "qemu/osdep.h"
> +#include "hw/arm/fdt.h"
> #include "hw/gpio/pl061.h"
> #include "hw/irq.h"
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> +#include "sysemu/device_tree.h"
>
> //#define DEBUG_PL061 1
>
> @@ -397,3 +399,43 @@ static void pl061_register_types(void)
> }
>
> type_init(pl061_register_types)
> +
> +/*
> + * pl061_create_fdt: Create a DT node for a PL061 GPIO controller
> + * @fdt: device tree blob
> + * @parent: name of the parent node
> + * @n_cells: value of #address-cells and #size-cells
> + * @base: base address of the controller's register block
> + * @size: size of the controller's register block
> + * @irq: interrupt number
> + * @clock: phandle of the apb-pclk clock
> + *
> + * Return value: a phandle referring to the created DT node.
> + *
> + * See the DT Binding Documentation in the Linux kernel source tree:
> + * Documentation/devicetree/bindings/gpio/pl061-gpio.yaml
> + */
> +uint32_t pl061_create_fdt(void *fdt, const char *parent, unsigned int
> n_cells,
> + hwaddr base, hwaddr size, int irq, uint32_t clock)
> +{
> +char *nodename = g_strdup_printf("%s/gpio@%" PRIx64, parent, base);
> +static const char compat[] = "arm,pl061\0arm,primecell";
> +uint32_t phandle = qemu_fdt_alloc_phandle(fdt);
> +
> +qemu_fdt_add_subnode(fdt, nodename);
> +qemu_fdt_setprop_sized_cells(fdt, nodename, "reg", n_cells, base,
> n_cells,
> + size);
> +qemu_fdt_setprop(fdt, nodename, "c
Re: [PATCH 09/11] target/mips: Always enable CONFIG_SEMIHOSTING
On 23/04/20 11:13, Philippe Mathieu-Daudé wrote: >> Let's then: >> >> 1) allow multiply-defined variables (just remove the "if" from >> do_declaration) > Apparently not needed with 2) Since "config SEMIHOSTING" is in hw/semihosting/Kconfig, it should be needed no? >> 2) do >> >> config SEMIHOSTING >> default y if TCG >> >> in target/arm/Kconfig. > You rock! >
Re: [PATCH QEMU v2 3/5] Add a GPIO backend using libgpiod
On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> Add a GPIO controller backend, to connect virtual GPIOs on the guest to
> physical GPIOs on the host. This allows the guest to control any
> external device connected to the physical GPIOs.
>
> Features and limitations:
> - The backend uses libgpiod on Linux,
> - For now only GPIO outputs are supported,
> - The number of GPIO lines mapped is limited to the number of GPIO
> lines available on the virtual GPIO controller.
>
> Future work:
> - GPIO inputs,
> - GPIO line configuration,
> - Optimizations for controlling multiple GPIO lines at once,
> - ...
>
> Signed-off-by: Geert Uytterhoeven
> ---
> v2:
> - Drop vgpios and gpios parameters, and map the full gpiochip instead,
> - Replace hardcoded PL061 instance by multiple dynamic instances,
> registered through qemu_gpiodev_add().
> ---
> MAINTAINERS | 6 +++
> backends/Makefile.objs | 2 +
> backends/gpiodev.c | 94
> configure| 28
> include/sysemu/gpiodev.h | 12 +
> 5 files changed, 142 insertions(+)
> create mode 100644 backends/gpiodev.c
> create mode 100644 include/sysemu/gpiodev.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e760f65270d29d5d..a70af47430083d14 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -607,6 +607,12 @@ F: include/hw/arm/digic.h
> F: hw/*/digic*
> F: include/hw/*/digic*
>
> +GPIO device backend
> +M: Geert Uytterhoeven
> +S: Supported
> +F: backends/gpiodev.c
> +F: include/sysemu/gpiodev.h
> +
> Goldfish RTC
> M: Anup Patel
> M: Alistair Francis
> diff --git a/backends/Makefile.objs b/backends/Makefile.objs
> index 28a847cd571d96ed..ee658e797454119a 100644
> --- a/backends/Makefile.objs
> +++ b/backends/Makefile.objs
> @@ -21,3 +21,5 @@ common-obj-$(CONFIG_LINUX) += hostmem-memfd.o
> common-obj-$(CONFIG_GIO) += dbus-vmstate.o
> dbus-vmstate.o-cflags = $(GIO_CFLAGS)
> dbus-vmstate.o-libs = $(GIO_LIBS)
> +
> +common-obj-$(CONFIG_GPIODEV) += gpiodev.o
> diff --git a/backends/gpiodev.c b/backends/gpiodev.c
> new file mode 100644
> index ..df1bd0113c7b2985
> --- /dev/null
> +++ b/backends/gpiodev.c
> @@ -0,0 +1,94 @@
> +/*
> + * QEMU GPIO Backend
> + *
> + * Copyright (C) 2018-2020 Glider bv
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include
probably not needed.
> +#include
Please move this one...
> +
> +#include "qemu/osdep.h"
> +#include "qemu/config-file.h"
> +#include "qemu/cutils.h"
> +#include "qemu/error-report.h"
> +#include "qemu/module.h"
> +#include "qemu/option.h"
> +#include "qapi/error.h"
> +
> +#include "sysemu/gpiodev.h"
> +
> +#include "hw/irq.h"
> +#include "hw/qdev-core.h"
... here:
#include
> +
> +static void gpiodev_irq_handler(void *opaque, int n, int level)
> +{
> +struct gpiod_line *line = opaque;
> +int status;
> +
> +status = gpiod_line_set_value(line, level);
> +if (status < 0) {
> +struct gpiod_chip *chip = gpiod_line_get_chip(line);
> +
> +error_report("%s/%s: Cannot set GPIO line %u: %s",
> + gpiod_chip_name(chip), gpiod_chip_label(chip),
> + gpiod_line_offset(line), strerror(errno));
> +}
> +}
> +
> +static int gpiodev_map_line(DeviceState *dev, struct gpiod_chip *chip,
> +unsigned int gpio, Error **errp)
> +{
> +struct gpiod_line *line;
> +qemu_irq irq;
> +int status;
> +
> +line = gpiod_chip_get_line(chip, gpio);
> +if (!line) {
> +error_setg(errp, "Cannot obtain GPIO line %u: %s", gpio,
> + strerror(errno));
> +return -1;
> +}
> +
> +status = gpiod_line_request_output(line, "qemu", 0);
> +if (status < 0) {
> +error_setg(errp, "Cannot request GPIO line %u for output: %s", gpio,
> + strerror(errno));
> +return status;
> +}
> +
> +irq = qemu_allocate_irq(gpiodev_irq_handler, line, 0);
> +qdev_connect_gpio_out(dev, gpio, irq);
> +return 0;
> +}
> +
> +void qemu_gpiodev_add(DeviceState *dev, const char *name, unsigned int
> maxgpio,
> + Error **errp)
> +{
> +struct gpiod_chip *chip;
> +unsigned int i, n;
> +int status;
> +
> +chip = gpiod_chip_open_lookup(name);
> +if (!chip) {
> +error_setg(errp, "Cannot open GPIO chip %s: %s", name,
> + strerror(errno));
> +return;
> +}
> +
> +n = gpiod_chip_num_lines(chip);
> +if (n > maxgpio) {
> +warn_report("Last %u GPIO line(s) will not be mapped", n - maxgpio);
> +n = maxgpio;
> +}
> +
> +for (i = 0; i < n; i++) {
> +status = gpiodev_map_line(dev, chip, i, errp);
> +if (status < 0) {
> +return;
> +}
> +}
> +
> +info_report("Mapped %u GPIO lines", n);
> +}
> diff --git a/configure b/configure
> index 23b5e93752b6a259..8b133402ef727c8e 100755
> -
Re: [PATCH QEMU v2 4/5] ARM: PL061: Add gpiodev support
On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> Make the PL061 GPIO controller user-creatable, and allow the user to tie
> a newly created instance to a gpiochip on the host.
>
> To create a new GPIO controller, the QEMU command line must be augmented
> with:
>
> -device pl061,host=
>
> with the name or label of the gpiochip on the host.
>
> Signed-off-by: Geert Uytterhoeven
> ---
> v2:
> - New.
> ---
> hw/gpio/pl061.c | 35 +++
> qemu-options.hx | 9 +
> 2 files changed, 44 insertions(+)
>
> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
> index 74ba733a8a5e8ca5..98204f9a586ae8c8 100644
> --- a/hw/gpio/pl061.c
> +++ b/hw/gpio/pl061.c
> @@ -12,11 +12,14 @@
> #include "hw/arm/fdt.h"
> #include "hw/gpio/pl061.h"
> #include "hw/irq.h"
> +#include "hw/qdev-properties.h"
> #include "hw/sysbus.h"
> #include "migration/vmstate.h"
> +#include "qapi/error.h"
> #include "qemu/log.h"
> #include "qemu/module.h"
> #include "sysemu/device_tree.h"
> +#include "sysemu/gpiodev.h"
>
> //#define DEBUG_PL061 1
>
> @@ -41,6 +44,9 @@ static const uint8_t pl061_id_luminary[12] =
> typedef struct PL061State {
> SysBusDevice parent_obj;
>
> +#ifdef CONFIG_GPIODEV
> +char *host;
> +#endif
> MemoryRegion iomem;
> uint32_t locked;
> uint32_t data;
> @@ -370,10 +376,39 @@ static void pl061_init(Object *obj)
> qdev_init_gpio_out(dev, s->out, 8);
Not related to this patch, but we should replace this 8 magic value by a
proper definition...
> }
>
> +#ifdef CONFIG_GPIODEV
> +static Property pl061_properties[] = {
> +DEFINE_PROP_STRING("host", PL061State, host),
> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void pl061_realize(DeviceState *dev, Error **errp)
> +{
> +PL061State *s = PL061(dev);
> +
> +if (!dev->opts) {
> +/* Not created by user */
> +return;
> +}
> +
> +if (!s->host) {
> +error_setg(errp, "'host' property is required");
> +return;
> +}
> +
> +qemu_gpiodev_add(dev, s->host, 8, errp);
> +}
> +#endif /* CONFIG_GPIODEV */
> +
> static void pl061_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> +#ifdef CONFIG_GPIODEV
> +device_class_set_props(dc, pl061_properties);
> +dc->realize = pl061_realize;
> +dc->user_creatable = true;
> +#endif
> dc->vmsd = &vmstate_pl061;
> dc->reset = &pl061_reset;
> }
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 292d4e7c0cef6097..182de7fb63923b38 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -875,6 +875,15 @@ SRST
> ``-device isa-ipmi-bt,bmc=id[,ioport=val][,irq=val]``
> Like the KCS interface, but defines a BT interface. The default port
> is 0xe4 and the default interrupt is 5.
> +
> +#ifdef CONFIG_GPIODEV
> +``-device pl061,host=gpiochip``
> +Add a PL061 GPIO controller, and map its virtual GPIO lines to a GPIO
> +controller on the host.
> +
> +``host=gpiochip``
> +The name or label of the GPIO controller on the host.
> +#endif
> ERST
>
> DEF("name", HAS_ARG, QEMU_OPTION_name,
>
Instead of restricting this to the pl061, it would be cleaner you add a
GPIO_PLUGGABLE_INTERFACE (or GPIO_BINDABLE_INTERFACE or better name),
and have TYPE_PL061 implement it.
Re: [PATCH QEMU v2 0/5] Add a GPIO backend
Patchew URL: https://patchew.org/QEMU/20200423090118.11199-1-geert+rene...@glider.be/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === CC hw/9pfs/9p-synth.o CC hw/9pfs/9p-proxy.o Warning, treated as error: /tmp/qemu-test/src/docs/../qemu-options.hx:881:Unexpected indentation. CC hw/9pfs/xen-9p-backend.o Warning, treated as error: /tmp/qemu-test/src/docs/../qemu-options.hx:881:Unexpected indentation. CC hw/acpi/core.o CC hw/acpi/piix4.o CC hw/acpi/pcihp.o CC hw/acpi/ich9.o CC hw/acpi/tco.o make: *** [Makefile:1104: docs/system/index.html] Error 2 make: *** Waiting for unfinished jobs make: *** [Makefile:1115: .docs_system_qemu.1_docs_system_qemu-block-drivers.7_docs_system_qemu-cpu-models.7.sentinel.] Error 2 make: *** Deleting file '.docs_system_qemu.1_docs_system_qemu-block-drivers.7_docs_system_qemu-cpu-models.7.sentinel.' Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=79991128d80240f382279e2c059f43f2', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-amlfs24e/src/docker-src.2020-04-23-05.29.38.22661:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=79991128d80240f382279e2c059f43f2 make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-amlfs24e/src' make: *** [docker-run-test-debug@fedora] Error 2 real4m43.053s user0m5.106s The full log is available at http://patchew.org/logs/20200423090118.11199-1-geert+rene...@glider.be/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 26/36] tcg: Add load_dest parameter to GVecGen2
Richard Henderson writes:
> We have this same parameter for GVecGen2i, GVecGen3,
> and GVecGen3i. This will make some SVE2 insns easier
> to parameterize.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
> ---
> include/tcg/tcg-op-gvec.h | 2 ++
> tcg/tcg-op-gvec.c | 45 ---
> 2 files changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/include/tcg/tcg-op-gvec.h b/include/tcg/tcg-op-gvec.h
> index d89f91f40e..cea6497341 100644
> --- a/include/tcg/tcg-op-gvec.h
> +++ b/include/tcg/tcg-op-gvec.h
> @@ -109,6 +109,8 @@ typedef struct {
> uint8_t vece;
> /* Prefer i64 to v64. */
> bool prefer_i64;
> +/* Load dest as a 2nd source operand. */
> +bool load_dest;
> } GVecGen2;
>
> typedef struct {
> diff --git a/tcg/tcg-op-gvec.c b/tcg/tcg-op-gvec.c
> index 43cac1a0bf..049a55e700 100644
> --- a/tcg/tcg-op-gvec.c
> +++ b/tcg/tcg-op-gvec.c
> @@ -663,17 +663,22 @@ static void expand_clr(uint32_t dofs, uint32_t maxsz)
>
> /* Expand OPSZ bytes worth of two-operand operations using i32 elements. */
> static void expand_2_i32(uint32_t dofs, uint32_t aofs, uint32_t oprsz,
> - void (*fni)(TCGv_i32, TCGv_i32))
> + bool load_dest, void (*fni)(TCGv_i32, TCGv_i32))
> {
> TCGv_i32 t0 = tcg_temp_new_i32();
> +TCGv_i32 t1 = tcg_temp_new_i32();
> uint32_t i;
>
> for (i = 0; i < oprsz; i += 4) {
> tcg_gen_ld_i32(t0, cpu_env, aofs + i);
> -fni(t0, t0);
> -tcg_gen_st_i32(t0, cpu_env, dofs + i);
> +if (load_dest) {
> +tcg_gen_ld_i32(t1, cpu_env, dofs + i);
> +}
> +fni(t1, t0);
> +tcg_gen_st_i32(t1, cpu_env, dofs + i);
> }
> tcg_temp_free_i32(t0);
> +tcg_temp_free_i32(t1);
> }
>
> static void expand_2i_i32(uint32_t dofs, uint32_t aofs, uint32_t oprsz,
> @@ -793,17 +798,22 @@ static void expand_4_i32(uint32_t dofs, uint32_t aofs,
> uint32_t bofs,
>
> /* Expand OPSZ bytes worth of two-operand operations using i64 elements. */
> static void expand_2_i64(uint32_t dofs, uint32_t aofs, uint32_t oprsz,
> - void (*fni)(TCGv_i64, TCGv_i64))
> + bool load_dest, void (*fni)(TCGv_i64, TCGv_i64))
> {
> TCGv_i64 t0 = tcg_temp_new_i64();
> +TCGv_i64 t1 = tcg_temp_new_i64();
> uint32_t i;
>
> for (i = 0; i < oprsz; i += 8) {
> tcg_gen_ld_i64(t0, cpu_env, aofs + i);
> -fni(t0, t0);
> -tcg_gen_st_i64(t0, cpu_env, dofs + i);
> +if (load_dest) {
> +tcg_gen_ld_i64(t1, cpu_env, dofs + i);
> +}
> +fni(t1, t0);
> +tcg_gen_st_i64(t1, cpu_env, dofs + i);
> }
> tcg_temp_free_i64(t0);
> +tcg_temp_free_i64(t1);
> }
>
> static void expand_2i_i64(uint32_t dofs, uint32_t aofs, uint32_t oprsz,
> @@ -924,17 +934,23 @@ static void expand_4_i64(uint32_t dofs, uint32_t aofs,
> uint32_t bofs,
> /* Expand OPSZ bytes worth of two-operand operations using host vectors. */
> static void expand_2_vec(unsigned vece, uint32_t dofs, uint32_t aofs,
> uint32_t oprsz, uint32_t tysz, TCGType type,
> + bool load_dest,
> void (*fni)(unsigned, TCGv_vec, TCGv_vec))
> {
> TCGv_vec t0 = tcg_temp_new_vec(type);
> +TCGv_vec t1 = tcg_temp_new_vec(type);
> uint32_t i;
>
> for (i = 0; i < oprsz; i += tysz) {
> tcg_gen_ld_vec(t0, cpu_env, aofs + i);
> -fni(vece, t0, t0);
> -tcg_gen_st_vec(t0, cpu_env, dofs + i);
> +if (load_dest) {
> +tcg_gen_ld_vec(t1, cpu_env, dofs + i);
> +}
> +fni(vece, t1, t0);
> +tcg_gen_st_vec(t1, cpu_env, dofs + i);
> }
> tcg_temp_free_vec(t0);
> +tcg_temp_free_vec(t1);
> }
>
> /* Expand OPSZ bytes worth of two-vector operands and an immediate operand
> @@ -1088,7 +1104,8 @@ void tcg_gen_gvec_2(uint32_t dofs, uint32_t aofs,
> * that e.g. size == 80 would be expanded with 2x32 + 1x16.
> */
> some = QEMU_ALIGN_DOWN(oprsz, 32);
> -expand_2_vec(g->vece, dofs, aofs, some, 32, TCG_TYPE_V256, g->fniv);
> +expand_2_vec(g->vece, dofs, aofs, some, 32, TCG_TYPE_V256,
> + g->load_dest, g->fniv);
> if (some == oprsz) {
> break;
> }
> @@ -1098,17 +1115,19 @@ void tcg_gen_gvec_2(uint32_t dofs, uint32_t aofs,
> maxsz -= some;
> /* fallthru */
> case TCG_TYPE_V128:
> -expand_2_vec(g->vece, dofs, aofs, oprsz, 16, TCG_TYPE_V128, g->fniv);
> +expand_2_vec(g->vece, dofs, aofs, oprsz, 16, TCG_TYPE_V128,
> + g->load_dest, g->fniv);
> break;
> case TCG_TYPE_V64:
> -expand_2_vec(g->vece, dofs, aofs, oprsz, 8, TCG_TYPE_V64, g->fniv);
> +expand_2_vec(g->vece, dofs, aofs, oprsz, 8, TCG_TYPE_V64,
> +
Re: [PATCH v2 27/36] tcg: Fix integral argument type to tcg_gen_rot[rl]i_i{32,64}
Richard Henderson writes:
> For the benefit of compatibility of function pointer types,
> we have standardized on int32_t and int64_t as the integral
> argument to tcg expanders.
>
> We converted most of them in 474b2e8f0f7, but missed the rotates.
>
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
> ---
> include/tcg/tcg-op.h | 8
> tcg/tcg-op.c | 16
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/include/tcg/tcg-op.h b/include/tcg/tcg-op.h
> index a39eb13ff0..b07bf7b524 100644
> --- a/include/tcg/tcg-op.h
> +++ b/include/tcg/tcg-op.h
> @@ -298,9 +298,9 @@ void tcg_gen_ctzi_i32(TCGv_i32 ret, TCGv_i32 arg1,
> uint32_t arg2);
> void tcg_gen_clrsb_i32(TCGv_i32 ret, TCGv_i32 arg);
> void tcg_gen_ctpop_i32(TCGv_i32 a1, TCGv_i32 a2);
> void tcg_gen_rotl_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2);
> -void tcg_gen_rotli_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
> +void tcg_gen_rotli_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
> void tcg_gen_rotr_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2);
> -void tcg_gen_rotri_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2);
> +void tcg_gen_rotri_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2);
> void tcg_gen_deposit_i32(TCGv_i32 ret, TCGv_i32 arg1, TCGv_i32 arg2,
> unsigned int ofs, unsigned int len);
> void tcg_gen_deposit_z_i32(TCGv_i32 ret, TCGv_i32 arg,
> @@ -490,9 +490,9 @@ void tcg_gen_ctzi_i64(TCGv_i64 ret, TCGv_i64 arg1,
> uint64_t arg2);
> void tcg_gen_clrsb_i64(TCGv_i64 ret, TCGv_i64 arg);
> void tcg_gen_ctpop_i64(TCGv_i64 a1, TCGv_i64 a2);
> void tcg_gen_rotl_i64(TCGv_i64 ret, TCGv_i64 arg1, TCGv_i64 arg2);
> -void tcg_gen_rotli_i64(TCGv_i64 ret, TCGv_i64 arg1, unsigned arg2);
> +void tcg_gen_rotli_i64(TCGv_i64 ret, TCGv_i64 arg1, int64_t arg2);
> void tcg_gen_rotr_i64(TCGv_i64 ret, TCGv_i64 arg1, TCGv_i64 arg2);
> -void tcg_gen_rotri_i64(TCGv_i64 ret, TCGv_i64 arg1, unsigned arg2);
> +void tcg_gen_rotri_i64(TCGv_i64 ret, TCGv_i64 arg1, int64_t arg2);
> void tcg_gen_deposit_i64(TCGv_i64 ret, TCGv_i64 arg1, TCGv_i64 arg2,
> unsigned int ofs, unsigned int len);
> void tcg_gen_deposit_z_i64(TCGv_i64 ret, TCGv_i64 arg,
> diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
> index 07eb661a07..202d8057c5 100644
> --- a/tcg/tcg-op.c
> +++ b/tcg/tcg-op.c
> @@ -516,9 +516,9 @@ void tcg_gen_rotl_i32(TCGv_i32 ret, TCGv_i32 arg1,
> TCGv_i32 arg2)
> }
> }
>
> -void tcg_gen_rotli_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2)
> +void tcg_gen_rotli_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2)
> {
> -tcg_debug_assert(arg2 < 32);
> +tcg_debug_assert(arg2 >= 0 && arg2 < 32);
> /* some cases can be optimized here */
> if (arg2 == 0) {
> tcg_gen_mov_i32(ret, arg1);
> @@ -554,9 +554,9 @@ void tcg_gen_rotr_i32(TCGv_i32 ret, TCGv_i32 arg1,
> TCGv_i32 arg2)
> }
> }
>
> -void tcg_gen_rotri_i32(TCGv_i32 ret, TCGv_i32 arg1, unsigned arg2)
> +void tcg_gen_rotri_i32(TCGv_i32 ret, TCGv_i32 arg1, int32_t arg2)
> {
> -tcg_debug_assert(arg2 < 32);
> +tcg_debug_assert(arg2 >= 0 && arg2 < 32);
> /* some cases can be optimized here */
> if (arg2 == 0) {
> tcg_gen_mov_i32(ret, arg1);
> @@ -1949,9 +1949,9 @@ void tcg_gen_rotl_i64(TCGv_i64 ret, TCGv_i64 arg1,
> TCGv_i64 arg2)
> }
> }
>
> -void tcg_gen_rotli_i64(TCGv_i64 ret, TCGv_i64 arg1, unsigned arg2)
> +void tcg_gen_rotli_i64(TCGv_i64 ret, TCGv_i64 arg1, int64_t arg2)
> {
> -tcg_debug_assert(arg2 < 64);
> +tcg_debug_assert(arg2 >= 0 && arg2 < 64);
> /* some cases can be optimized here */
> if (arg2 == 0) {
> tcg_gen_mov_i64(ret, arg1);
> @@ -1986,9 +1986,9 @@ void tcg_gen_rotr_i64(TCGv_i64 ret, TCGv_i64 arg1,
> TCGv_i64 arg2)
> }
> }
>
> -void tcg_gen_rotri_i64(TCGv_i64 ret, TCGv_i64 arg1, unsigned arg2)
> +void tcg_gen_rotri_i64(TCGv_i64 ret, TCGv_i64 arg1, int64_t arg2)
> {
> -tcg_debug_assert(arg2 < 64);
> +tcg_debug_assert(arg2 >= 0 && arg2 < 64);
> /* some cases can be optimized here */
> if (arg2 == 0) {
> tcg_gen_mov_i64(ret, arg1);
--
Alex Bennée
Re: [PATCH QEMU v2 0/5] Add a GPIO backend
Patchew URL: https://patchew.org/QEMU/20200423090118.11199-1-geert+rene...@glider.be/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === CC hw/acpi/generic_event_device.o CC hw/acpi/hmat.o Warning, treated as error: /tmp/qemu-test/src/docs/../qemu-options.hx:881:Unexpected indentation. CC hw/acpi/acpi_interface.o CC hw/acpi/bios-linker-loader.o --- CC hw/acpi/pci.o CC hw/acpi/ipmi.o CC hw/acpi/acpi-stub.o make: *** [Makefile:1115: .docs_system_qemu.1_docs_system_qemu-block-drivers.7_docs_system_qemu-cpu-models.7.sentinel.] Error 2 make: *** Deleting file '.docs_system_qemu.1_docs_system_qemu-block-drivers.7_docs_system_qemu-cpu-models.7.sentinel.' make: *** Waiting for unfinished jobs Warning, treated as error: /tmp/qemu-test/src/docs/../qemu-options.hx:881:Unexpected indentation. make: *** [Makefile:1104: docs/system/index.html] Error 2 Traceback (most recent call last): File "./tests/docker/docker.py", line 664, in sys.exit(main()) --- raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=db085d5fd0c44c1fa6016be6e323530d', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-ds0rnrhj/src/docker-src.2020-04-23-05.37.44.31543:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2. filter=--filter=label=com.qemu.instance.uuid=db085d5fd0c44c1fa6016be6e323530d make[1]: *** [docker-run] Error 1 make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ds0rnrhj/src' make: *** [docker-run-test-mingw@fedora] Error 2 real2m45.699s user0m7.476s The full log is available at http://patchew.org/logs/20200423090118.11199-1-geert+rene...@glider.be/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH QEMU v2 3/5] Add a GPIO backend using libgpiod
Hi Philippe,
Thanks for your comments!
On Thu, Apr 23, 2020 at 11:28 AM Philippe Mathieu-Daudé wrote:
> On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
> > Add a GPIO controller backend, to connect virtual GPIOs on the guest to
> > physical GPIOs on the host. This allows the guest to control any
> > external device connected to the physical GPIOs.
> >
> > Features and limitations:
> > - The backend uses libgpiod on Linux,
> > - For now only GPIO outputs are supported,
> > - The number of GPIO lines mapped is limited to the number of GPIO
> > lines available on the virtual GPIO controller.
> >
> > Future work:
> > - GPIO inputs,
> > - GPIO line configuration,
> > - Optimizations for controlling multiple GPIO lines at once,
> > - ...
> >
> > Signed-off-by: Geert Uytterhoeven
> > --- /dev/null
> > +++ b/backends/gpiodev.c
> > @@ -0,0 +1,94 @@
> > +/*
> > + * QEMU GPIO Backend
> > + *
> > + * Copyright (C) 2018-2020 Glider bv
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include
>
> probably not needed.
It is indeed included by one of the other header files.
What is the QEMU policy w.r.t. that?
>
> > +#include
>
> Please move this one...
>
> > +
> > +#include "qemu/osdep.h"
> > +#include "qemu/config-file.h"
> > +#include "qemu/cutils.h"
I forgot to remove the two above...
> > +#include "qemu/error-report.h"
> > +#include "qemu/module.h"
> > +#include "qemu/option.h"
... and the two above.
> > +#include "qapi/error.h"
> > +
> > +#include "sysemu/gpiodev.h"
> > +
> > +#include "hw/irq.h"
> > +#include "hw/qdev-core.h"
>
> ... here:
>
> #include
OK.
> > --- a/configure
> > +++ b/configure
> > @@ -509,6 +509,7 @@ libpmem=""
> > default_devices="yes"
> > plugins="no"
> > fuzzing="no"
> > +gpio=""
>
> Maybe name this feature 'libgpiod'?
Makes sense.
> >
> > supported_cpu="no"
> > supported_os="no"
> > @@ -1601,6 +1602,10 @@ for opt do
> >;;
> >--gdb=*) gdb_bin="$optarg"
> >;;
> > + --disable-gpio) gpio="no"
> > + ;;
> > + --enable-gpio) gpio="yes"
>
> Ditto: --enable-libgpiod, because else it seems rather confusing.
OK.
> > --- /dev/null
> > +++ b/include/sysemu/gpiodev.h
> > @@ -0,0 +1,12 @@
> > +/*
> > + * QEMU GPIO Backend
> > + *
> > + * Copyright (C) 2018-2020 Glider bv
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/typedefs.h"
>
> "qemu/typedefs.h" not needed in includes.
While removing that works, it does mean the header file is no longer
self-contained:
include/sysemu/gpiodev.h:10:23: error: unknown type name ‘DeviceState’
> > +
> > +void qemu_gpiodev_add(DeviceState *dev, const char *name, unsigned int
> > maxgpio,
> > + Error **errp);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v5 1/9] block: Add flags to BlockDriver.bdrv_co_truncate()
On 22.04.20 17:21, Kevin Wolf wrote: > This adds a new BdrvRequestFlags parameter to the .bdrv_co_truncate() > driver callbacks, and a supported_truncate_flags field in > BlockDriverState that allows drivers to advertise support for request > flags in the context of truncate. > > For now, we always pass 0 and no drivers declare support for any flag. > > Signed-off-by: Kevin Wolf > Reviewed-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Alberto Garcia > --- > include/block/block_int.h | 10 +- > block/crypto.c | 3 ++- > block/file-posix.c | 2 +- > block/file-win32.c | 2 +- > block/gluster.c | 1 + > block/io.c | 8 +++- > block/iscsi.c | 2 +- > block/nfs.c | 3 ++- > block/qcow2.c | 2 +- > block/qed.c | 1 + > block/raw-format.c | 2 +- > block/rbd.c | 1 + > block/sheepdog.c| 4 ++-- > block/ssh.c | 2 +- > tests/test-block-iothread.c | 3 ++- > 15 files changed, 33 insertions(+), 13 deletions(-) (I know I haven’t complained before, so *shrug*, but I wonder now whether it actually makes sense to have the same BdrvRequestFlags for all request types. Or why we have the same flags type for read, write, and zero-write already.) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [PATCH 09/11] target/mips: Always enable CONFIG_SEMIHOSTING
On 23/04/20 10:43, Philippe Mathieu-Daudé wrote:
>>
>> You can add CONFIG_SEMIHOSTING=y directly in the Kconfig file.
>
> I didn't know because it is not documented and no examples, but I the
> code is here:
>
> # assignment_var: ID (starting with "CONFIG_")
> def parse_assignment_var(self):
> if self.tok == TOK_ID:
> val = self.val
> if not val.startswith("CONFIG_"):
> raise KconfigParserError(self,
> 'Expected identifier starting with
> "CONFIG_"', TOK_NONE)
> self.get_token()
> return self.data.do_var(val[7:])
> else:
> raise KconfigParserError(self, 'Expected identifier')
>
> # assignment: var EQUAL y_or_n
> def parse_assignment(self):
> var = self.parse_assignment_var()
> if self.tok != TOK_EQUAL:
> raise KconfigParserError(self, 'Expected "="')
> self.get_token()
> self.data.do_assignment(var, self.parse_y_or_n())
>
> Thanks!
Well yeah, it's a bit of a hack and simply the simplest way to implement
it. If it turns out that there are other ways to achieve what you need,
it's better.
Paolo
Re: [PATCH v5 2/9] block: Add flags to bdrv(_co)_truncate()
On 22.04.20 17:21, Kevin Wolf wrote: > Now that block drivers can support flags for .bdrv_co_truncate, expose > the parameter in the node level interfaces bdrv_co_truncate() and > bdrv_truncate(). > > Signed-off-by: Kevin Wolf > Reviewed-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Alberto Garcia > --- > include/block/block.h | 5 +++-- > block/block-backend.c | 2 +- > block/crypto.c | 2 +- > block/io.c | 12 +++- > block/parallels.c | 6 +++--- > block/qcow.c| 4 ++-- > block/qcow2-refcount.c | 2 +- > block/qcow2.c | 15 +-- > block/raw-format.c | 2 +- > block/vhdx-log.c| 2 +- > block/vhdx.c| 2 +- > block/vmdk.c| 2 +- > tests/test-block-iothread.c | 6 +++--- > 13 files changed, 34 insertions(+), 28 deletions(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [PATCH QEMU v2 3/5] Add a GPIO backend using libgpiod
On 4/23/20 11:41 AM, Geert Uytterhoeven wrote:
> Hi Philippe,
>
> Thanks for your comments!
>
> On Thu, Apr 23, 2020 at 11:28 AM Philippe Mathieu-Daudé
> wrote:
>> On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
>>> Add a GPIO controller backend, to connect virtual GPIOs on the guest to
>>> physical GPIOs on the host. This allows the guest to control any
>>> external device connected to the physical GPIOs.
>>>
>>> Features and limitations:
>>> - The backend uses libgpiod on Linux,
>>> - For now only GPIO outputs are supported,
>>> - The number of GPIO lines mapped is limited to the number of GPIO
>>> lines available on the virtual GPIO controller.
>>>
>>> Future work:
>>> - GPIO inputs,
>>> - GPIO line configuration,
>>> - Optimizations for controlling multiple GPIO lines at once,
>>> - ...
>>>
>>> Signed-off-by: Geert Uytterhoeven
>
>>> --- /dev/null
>>> +++ b/backends/gpiodev.c
>>> @@ -0,0 +1,94 @@
>>> +/*
>>> + * QEMU GPIO Backend
>>> + *
>>> + * Copyright (C) 2018-2020 Glider bv
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include
>>
>> probably not needed.
>
> It is indeed included by one of the other header files.
> What is the QEMU policy w.r.t. that?
See CODING_STYLE.rst:
Include directives
--
Order include directives as follows:
.. code-block:: c
#include "qemu/osdep.h" /* Always first... */
#include <...> /* then system headers... */
#include "..." /* and finally QEMU headers. */
The "qemu/osdep.h" header contains preprocessor macros that affect the
behavior
of core system headers like . It must be the first include so
that
core system headers included by external libraries get the preprocessor
macros
that QEMU depends on.
>
>>
>>> +#include
>>
>> Please move this one...
>>
>>> +
>>> +#include "qemu/osdep.h"
>>> +#include "qemu/config-file.h"
>>> +#include "qemu/cutils.h"
>
> I forgot to remove the two above...
>
>>> +#include "qemu/error-report.h"
>>> +#include "qemu/module.h"
>>> +#include "qemu/option.h"
>
> ... and the two above.
>
>>> +#include "qapi/error.h"
>>> +
>>> +#include "sysemu/gpiodev.h"
>>> +
>>> +#include "hw/irq.h"
>>> +#include "hw/qdev-core.h"
>>
>> ... here:
>>
>> #include
>
> OK.
>
>>> --- a/configure
>>> +++ b/configure
>>> @@ -509,6 +509,7 @@ libpmem=""
>>> default_devices="yes"
>>> plugins="no"
>>> fuzzing="no"
>>> +gpio=""
>>
>> Maybe name this feature 'libgpiod'?
>
> Makes sense.
>
>>>
>>> supported_cpu="no"
>>> supported_os="no"
>>> @@ -1601,6 +1602,10 @@ for opt do
>>>;;
>>>--gdb=*) gdb_bin="$optarg"
>>>;;
>>> + --disable-gpio) gpio="no"
>>> + ;;
>>> + --enable-gpio) gpio="yes"
>>
>> Ditto: --enable-libgpiod, because else it seems rather confusing.
>
> OK.
>
>>> --- /dev/null
>>> +++ b/include/sysemu/gpiodev.h
>>> @@ -0,0 +1,12 @@
>>> +/*
>>> + * QEMU GPIO Backend
>>> + *
>>> + * Copyright (C) 2018-2020 Glider bv
>>> + *
>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>> + */
>>> +
>>> +#include "qemu/typedefs.h"
>>
>> "qemu/typedefs.h" not needed in includes.
>
> While removing that works, it does mean the header file is no longer
> self-contained:
>
> include/sysemu/gpiodev.h:10:23: error: unknown type name ‘DeviceState’
Odd, because your backends/gpiodev.c already has:
#include "hw/qdev-core.h"
>
>>> +
>>> +void qemu_gpiodev_add(DeviceState *dev, const char *name, unsigned int
>>> maxgpio,
>>> + Error **errp);
>
> Gr{oetje,eeting}s,
>
> Geert
>
Re: [PATCH QEMU v2 4/5] ARM: PL061: Add gpiodev support
On 4/23/20 11:33 AM, Philippe Mathieu-Daudé wrote:
> On 4/23/20 11:01 AM, Geert Uytterhoeven wrote:
>> Make the PL061 GPIO controller user-creatable, and allow the user to tie
>> a newly created instance to a gpiochip on the host.
>>
>> To create a new GPIO controller, the QEMU command line must be augmented
>> with:
>>
>> -device pl061,host=
>>
>> with the name or label of the gpiochip on the host.
>>
>> Signed-off-by: Geert Uytterhoeven
>> ---
>> v2:
>> - New.
>> ---
>> hw/gpio/pl061.c | 35 +++
>> qemu-options.hx | 9 +
>> 2 files changed, 44 insertions(+)
>>
>> diff --git a/hw/gpio/pl061.c b/hw/gpio/pl061.c
>> index 74ba733a8a5e8ca5..98204f9a586ae8c8 100644
>> --- a/hw/gpio/pl061.c
>> +++ b/hw/gpio/pl061.c
>> @@ -12,11 +12,14 @@
>> #include "hw/arm/fdt.h"
>> #include "hw/gpio/pl061.h"
>> #include "hw/irq.h"
>> +#include "hw/qdev-properties.h"
>> #include "hw/sysbus.h"
>> #include "migration/vmstate.h"
>> +#include "qapi/error.h"
>> #include "qemu/log.h"
>> #include "qemu/module.h"
>> #include "sysemu/device_tree.h"
>> +#include "sysemu/gpiodev.h"
>>
>> //#define DEBUG_PL061 1
>>
>> @@ -41,6 +44,9 @@ static const uint8_t pl061_id_luminary[12] =
>> typedef struct PL061State {
>> SysBusDevice parent_obj;
>>
>> +#ifdef CONFIG_GPIODEV
>> +char *host;
>> +#endif
>> MemoryRegion iomem;
>> uint32_t locked;
>> uint32_t data;
>> @@ -370,10 +376,39 @@ static void pl061_init(Object *obj)
>> qdev_init_gpio_out(dev, s->out, 8);
>
> Not related to this patch, but we should replace this 8 magic value by a
> proper definition...
>
>> }
>>
>> +#ifdef CONFIG_GPIODEV
>> +static Property pl061_properties[] = {
>> +DEFINE_PROP_STRING("host", PL061State, host),
>> +DEFINE_PROP_END_OF_LIST(),
>> +};
>> +
>> +static void pl061_realize(DeviceState *dev, Error **errp)
>> +{
>> +PL061State *s = PL061(dev);
>> +
>> +if (!dev->opts) {
>> +/* Not created by user */
>> +return;
>> +}
>> +
>> +if (!s->host) {
>> +error_setg(errp, "'host' property is required");
>> +return;
>> +}
>> +
>> +qemu_gpiodev_add(dev, s->host, 8, errp);
>> +}
>> +#endif /* CONFIG_GPIODEV */
>> +
>> static void pl061_class_init(ObjectClass *klass, void *data)
>> {
>> DeviceClass *dc = DEVICE_CLASS(klass);
>>
>> +#ifdef CONFIG_GPIODEV
>> +device_class_set_props(dc, pl061_properties);
>> +dc->realize = pl061_realize;
>> +dc->user_creatable = true;
>> +#endif
>> dc->vmsd = &vmstate_pl061;
>> dc->reset = &pl061_reset;
>> }
>> diff --git a/qemu-options.hx b/qemu-options.hx
>> index 292d4e7c0cef6097..182de7fb63923b38 100644
>> --- a/qemu-options.hx
>> +++ b/qemu-options.hx
>> @@ -875,6 +875,15 @@ SRST
>> ``-device isa-ipmi-bt,bmc=id[,ioport=val][,irq=val]``
>> Like the KCS interface, but defines a BT interface. The default port
>> is 0xe4 and the default interrupt is 5.
>> +
>> +#ifdef CONFIG_GPIODEV
>> +``-device pl061,host=gpiochip``
>> +Add a PL061 GPIO controller, and map its virtual GPIO lines to a GPIO
>> +controller on the host.
>> +
>> +``host=gpiochip``
>> +The name or label of the GPIO controller on the host.
>> +#endif
>> ERST
>>
>> DEF("name", HAS_ARG, QEMU_OPTION_name,
>>
>
> Instead of restricting this to the pl061, it would be cleaner you add a
> GPIO_PLUGGABLE_INTERFACE (or GPIO_BINDABLE_INTERFACE or better name),
> and have TYPE_PL061 implement it.
IOW your backend should consume devices implementing this generic interface.
Re: [PATCH v5 3/9] block-backend: Add flags to blk_truncate()
On 22.04.20 17:21, Kevin Wolf wrote: > Now that node level interface bdrv_truncate() supports passing request > flags to the block driver, expose this on the BlockBackend level, too. > > Signed-off-by: Kevin Wolf > Reviewed-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Alberto Garcia > --- > include/sysemu/block-backend.h | 2 +- > block.c| 3 ++- > block/block-backend.c | 4 ++-- > block/commit.c | 4 ++-- > block/crypto.c | 2 +- > block/mirror.c | 2 +- > block/qcow2.c | 4 ++-- > block/qed.c| 2 +- > block/vdi.c| 2 +- > block/vhdx.c | 4 ++-- > block/vmdk.c | 6 +++--- > block/vpc.c| 2 +- > blockdev.c | 2 +- > qemu-img.c | 2 +- > qemu-io-cmds.c | 2 +- > 15 files changed, 22 insertions(+), 21 deletions(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [PATCH] qemu-sockets: add abstract UNIX domain socket support
Hi
On Thu, Apr 23, 2020 at 4:48 AM xiaoqiang zhao wrote:
>
> unix_connect_saddr now support abstract address type
>
> By default qemu does not support abstract UNIX domain
> socket address. Add this ability to make qemu handy
> when abstract address is needed.
> Abstract address is marked by prefixing the address name with a '@'.
>
> Signed-off-by: xiaoqiang zhao
Reviewed-by: Marc-André Lureau
> ---
> util/qemu-sockets.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index bcc06d0e01..d4f02a6b1a 100644
> --- a/util/qemu-sockets.c
> +++ b/util/qemu-sockets.c
> @@ -939,6 +939,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr,
> Error **errp)
> struct sockaddr_un un;
> int sock, rc;
> size_t pathlen;
> +socklen_t serverlen;
>
> if (saddr->path == NULL) {
> error_setg(errp, "unix connect: no path specified");
> @@ -963,10 +964,18 @@ static int unix_connect_saddr(UnixSocketAddress *saddr,
> Error **errp)
> un.sun_family = AF_UNIX;
> memcpy(un.sun_path, saddr->path, pathlen);
>
> +if (saddr->path[0] == '@') {
> +un.sun_path[0] = '\0';
> +serverlen = strlen(saddr->path) + offsetof(struct sockaddr_un,
> sun_path);
> +}
> +else {
> +serverlen = sizeof(un);
> +}
> +
> /* connect to peer */
> do {
> rc = 0;
> -if (connect(sock, (struct sockaddr *) &un, sizeof(un)) < 0) {
> +if (connect(sock, (struct sockaddr *) &un, serverlen) < 0) {
> rc = -errno;
> }
> } while (rc == -EINTR);
> --
> 2.17.1
>
>
>
--
Marc-André Lureau
Re: Integration of qemu-img
On Thu, Apr 23, 2020 at 11:20 AM wrote: > this requires the user of the application to install qemu first right? > If this is the case then this is unfortunately not an option. The user shall > not be bothered with installing anything else then the tool. Hi Janine, Please use Reply-All to keep the email CC list in tact. That way qemu-devel@nongnu.org will receive our replies and the discussion will stay on the mailing list. Thanks! It's common for applications to consist of more than a single executable file. They could have shared libraries, data files, or other executables like qemu-img.exe. You can distribute qemu-img.exe together with your application as part of a zip file or installer. Regardless of whether you ship qemu-img.exe or build a library, please check QEMU's software license so that you can follow the terms of the GPL open source license. Stefan
[PATCH] Makefile: Let the 'help' target list the helper targets
List the name of the helper targets when calling 'make help', along with the tool targets: $ make help [...] Helper targets: fsdev/virtfs-proxy-helper - Build virtfs-proxy-helper scsi/qemu-pr-helper- Build qemu-pr-helper qemu-bridge-helper - Build qemu-bridge-helper vhost-user-gpu - Build vhost-user-gpu virtiofsd - Build virtiofsd Tools targets: qemu-ga- Build qemu-ga tool qemu-keymap- Build qemu-keymap tool elf2dmp- Build elf2dmp tool ivshmem-client - Build ivshmem-client tool ivshmem-server - Build ivshmem-server tool qemu-nbd - Build qemu-nbd tool qemu-storage-daemon- Build qemu-storage-daemon tool qemu-img - Build qemu-img tool qemu-io- Build qemu-io tool qemu-edid - Build qemu-edid tool Signed-off-by: Philippe Mathieu-Daudé --- configure | 5 +++-- Makefile | 9 +++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/configure b/configure index 23b5e93752..caf880c38e 100755 --- a/configure +++ b/configure @@ -6374,7 +6374,7 @@ if test "$softmmu" = yes ; then if test "$linux" = yes; then if test "$virtfs" != no && test "$cap_ng" = yes && test "$attr" = yes ; then virtfs=yes - tools="$tools fsdev/virtfs-proxy-helper\$(EXESUF)" + helpers="$helpers fsdev/virtfs-proxy-helper\$(EXESUF)" else if test "$virtfs" = yes; then error_exit "VirtFS requires libcap-ng devel and libattr devel" @@ -6389,7 +6389,7 @@ if test "$softmmu" = yes ; then fi mpath=no fi -tools="$tools scsi/qemu-pr-helper\$(EXESUF)" +helpers="$helpers scsi/qemu-pr-helper\$(EXESUF)" else if test "$virtfs" = yes; then error_exit "VirtFS is supported only on Linux" @@ -7630,6 +7630,7 @@ else QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/\$(ARCH) $QEMU_INCLUDES" fi +echo "HELPERS=$helpers" >> $config_host_mak echo "TOOLS=$tools" >> $config_host_mak echo "ROMS=$roms" >> $config_host_mak echo "MAKE=$make" >> $config_host_mak diff --git a/Makefile b/Makefile index 8a9113e666..021a0cd491 100644 --- a/Makefile +++ b/Makefile @@ -336,9 +336,9 @@ $(call set-vpath, $(SRC_PATH)) LIBS+=-lz $(LIBS_TOOLS) vhost-user-json-y = -HELPERS-y = +HELPERS-y = $(HELPERS) -HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) = qemu-bridge-helper$(EXESUF) +HELPERS-$(call land,$(CONFIG_SOFTMMU),$(CONFIG_LINUX)) += qemu-bridge-helper$(EXESUF) ifeq ($(CONFIG_LINUX)$(CONFIG_VIRGL)$(CONFIG_GBM)$(CONFIG_TOOLS),) HELPERS-y += vhost-user-gpu$(EXESUF) @@ -1255,6 +1255,11 @@ endif $(foreach t, $(TARGET_DIRS), \ $(call print-help-run,$(t)/all,Build for $(t));) \ echo '') + @$(if $(HELPERS-y), \ + echo 'Helper targets:'; \ + $(foreach t, $(HELPERS-y), \ + $(call print-help-run,$(t),Build $(shell basename $(t)));) \ + echo '') @$(if $(TOOLS), \ echo 'Tools targets:'; \ $(foreach t, $(TOOLS), \ -- 2.21.1
Re: [PATCH v2] qemu-sockets: add abstract UNIX domain socket support
在 2020/4/23 下午5:00, Daniel P. Berrangé 写道:
Adding Eric & Markus for QAPI modelling questions
On Thu, Apr 23, 2020 at 11:56:40AM +0800, xiaoqiang zhao wrote:
unix_connect_saddr now support abstract address type
By default qemu does not support abstract UNIX domain
socket address. Add this ability to make qemu handy
when abstract address is needed.
Was that a specific app you're using with QEMU that needs this ?
Thanks for your reply !
I once use qemu to connect a unix domain socket server (with abstract
type address written by android java code)
Abstract address is marked by prefixing the address name with a '@'.
For full support of the abstract namespace we would ned to allow
the "sun_path" to contain an arbitrary mix of NULs and non-NULs
characters, and allow connect() @addrlen to be an arbitrary size.
This patch only allows a single initial NUL, and reqiures @addrlen
to be the full size of sun_path, padding with trailing NULs. This
limitation is impossible to lift with QEMU's current approach to
UNIX sockets, as it relies on passing around a NULL terminated
string, so there's no way to have embedded NULs. Since there's
no explicit length, we have to chooose between forcing the full
sun_path size as @addrlen, or forcing the string length as the
@addrlen value.
IIUC, socat makes the latter decision by default, but has a
flag to switch to the former.
[man socat]
unix-tightsocklen=[0|1]
On socket operations, pass a socket address length that does not
include the whole struct sockaddr_un record but (besides other compo‐
nents) only the relevant part of the filename or abstract string.
Default is 1.
[/man]
This actually is supported for both abstract and non-abstract
sockets, though IIUC this doesn't make a semantic difference
for non-abstract sockets.
The point is we have four possible combinations
NON-ABSTRACT + FULL SIZE
NON-ABSTRACT + MINIMAL SIZE (default)
ABSTRACT + FULL SIZE
ABSTRACT + MINIMAL SIZE (default)
With your patch doing the latter, it means QEMU supports
only two combinations
NON+ABSTRACT + FULL SIZE
ABSTRACT + MINIMAL SIZE
and also can't use "@somerealpath" for a non-abstract
socket, though admittedly this is unlikely.
Socat uses a special option to request use of abstract
sockets. eg ABSTRACT:somepath, and automatically adds
the leading NUL, so there's no need for a special "@"
character. This means that UNIX:@somepath still resolves
to a filesystem path and not a abstract socket path.
Finally, the patch as only added support for connect()
not listen(). I think if QEMU wants to support abstract
sockets we must do both, and also have unit tests added
to tests/test-util-sockets.c
Yes , I missed these parts.
The question is whether we're ok with this simple
approach in QEMU, or should do a full approach with
more explicit modelling.
Agree, more comments is welcome.
ie should we change QAPI thus:
{ 'struct': 'UnixSocketAddress',
'data': {
'path': 'str',
'tight': 'bool',
'abstract': 'bool' } }
where 'tight' is a flag indicating whether to set @addrlen
to the minimal string length, or the maximum sun_path length.
And 'abstract' indicates that we automagically add a leading
NUL.
This would *not* allow for NULs in the middle of path,
but I'm not so bothered about that, since I can't see that
being widely used. If we really did need that it could be
added via a 'base64': 'bool' flag, to indicate that @path
is base64 encoded and thus may contain NULs
From a CLI POV, this could be mapped to QAPI thus
* -chardev unix:somepath
@path==somepath
@tight==false
@abstract==false
* -chardev unix:somepath,tight
@path==somepath
@tight==true
@abstract==false
* -chardev unix-abstract:somepath
@path==somepath
@tight==false
@abstract==true
* -chardev unix-abstract:somepath,tight
@path==somepath
@tight==true
@abstract==true
Regards,
Daniel
Re: [PATCH v5 4/9] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate
On 22.04.20 17:21, Kevin Wolf wrote:
> If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling
> qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't
> undo any previous preallocation, but just adds the zero flag to all
> relevant L2 entries. If an external data file is in use, a write_zeroes
> request to the data file is made instead.
>
> Signed-off-by: Kevin Wolf
> ---
> block/qcow2.c | 30 ++
> 1 file changed, 30 insertions(+)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 9cfbdfc939..bd632405d1 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
[...]
> @@ -4214,6 +4215,35 @@ static int coroutine_fn
> qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> g_assert_not_reached();
> }
>
> +if ((flags & BDRV_REQ_ZERO_WRITE) && offset > old_length) {
> +uint64_t zero_start = QEMU_ALIGN_UP(old_length, s->cluster_size);
> +uint64_t zero_end = QEMU_ALIGN_UP(offset, s->cluster_size);
> +
> +/* Use zero clusters as much as we can */
> +ret = qcow2_cluster_zeroize(bs, zero_start, zero_end - zero_start,
> 0);
It’s kind of a pity that this changes the cluster mappings that were
established when using falloc/full preallocation already (i.e., they
become preallocated zero clusters then, so when writing to them, we need
COW again).
But falloc/full preallocation do not guarantee that the new data is
zero, so I suppose this is the only thing we can reasonably do.
I personally don’t really care about whether zero_end is aligned or not,
so either way:
Reviewed-by: Max Reitz
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v5 5/9] raw-format: Support BDRV_REQ_ZERO_WRITE for truncate
On 22.04.20 17:21, Kevin Wolf wrote: > The raw format driver can simply forward the flag and let its bs->file > child take care of actually providing the zeros. > > Signed-off-by: Kevin Wolf > --- > block/raw-format.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
AW: Integration of qemu-img
Hy again, okay so now we have an easy way out just in case. But I still want to build an DLL and/or a shared library for integration into the tool. I want the tool to be platform independent and I was already able to build qemu-img as cross build with mingw64. Does anybody have experience in building a qemu library or tried it already? The tool I want to integrate qemu in is published under GPL itself. And if I am able to build qemu as library I will share it with the community and everybody interested in having it. Best, Janine -Ursprüngliche Nachricht- Von: Stefan Hajnoczi Gesendet: Donnerstag, 23. April 2020 12:41 An: janine.schnei...@fau.de Cc: qemu-devel ; qemu block Betreff: Re: Integration of qemu-img On Thu, Apr 23, 2020 at 11:20 AM wrote: > this requires the user of the application to install qemu first right? > If this is the case then this is unfortunately not an option. The user > shall not be bothered with installing anything else then the tool. Hi Janine, Please use Reply-All to keep the email CC list in tact. That way qemu-devel@nongnu.org will receive our replies and the discussion will stay on the mailing list. Thanks! It's common for applications to consist of more than a single executable file. They could have shared libraries, data files, or other executables like qemu-img.exe. You can distribute qemu-img.exe together with your application as part of a zip file or installer. Regardless of whether you ship qemu-img.exe or build a library, please check QEMU's software license so that you can follow the terms of the GPL open source license. Stefan
Re: [PATCH v5 6/9] file-posix: Support BDRV_REQ_ZERO_WRITE for truncate
On 22.04.20 17:21, Kevin Wolf wrote: > For regular files, we always get BDRV_REQ_ZERO_WRITE behaviour from the > OS, so we can advertise the flag and just ignore it. > > Signed-off-by: Kevin Wolf > Reviewed-by: Vladimir Sementsov-Ogievskiy > Reviewed-by: Alberto Garcia > --- > block/file-posix.c | 4 > 1 file changed, 4 insertions(+) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: Integration of qemu-img
On Thu, Apr 23, 2020 at 12:53:48PM +0200, janine.schnei...@fau.de wrote: > Hy again, > > okay so now we have an easy way out just in case. > But I still want to build an DLL and/or a shared library for integration > into the tool. I want the tool to be platform independent and I was > already able to build qemu-img as cross build with mingw64. Does anybody > have experience in building a qemu library or tried it already? It has been discussed in the past, but general wasn't considered a viable, because any apps using it would have to be strictly licensed as GPLv2-only. This would prevent the library being used by anything that includes GPLv3 code, or obviously from closed source apps. This would seriously restrict how useful any library was. I would also note that QEMU disk code is not robust against malicously created disk images. It is possible to create images that inflict a denial of service in terms of memory and CPU usage. Thus if an application is handling disk images obtained from untrusted users, it is desirable for qemu-img to be a separate process, such that you can put strict resource limits on it as protection against DoS. > The tool I want to integrate qemu in is published under GPL itself. And > if I am able to build qemu as library I will share it with the community > and everybody interested in having it. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [PATCH v3] hw/char/pl011: Enable TxFIFO and async transmission
On Wed, 11 Mar 2020 at 04:09, Gavin Shan wrote:
>
> The depth of TxFIFO can be 1 or 16 depending on LCR[4]. The TxFIFO is
> disabled when its depth is 1. It's nice to have TxFIFO enabled if
> possible because more characters can be piled and transmitted at once,
> which would have less overhead. Besides, we can be blocked because of
> qemu_chr_fe_write_all(), which isn't nice.
>
> This enables TxFIFO if possible. On ther other hand, the asynchronous
> transmission is enabled if needed, as we did in hw/char/cadence_uart.c
>
> Signed-off-by: Gavin Shan
> ---
> v3: Use PL011() to do data type conversion
> Return G_SOURCE_REMOVE when the backend is disconnected in pl011_xmit()
> Drop parenthesis in the condition validating @size in pl011_write_fifo()
> ---
> hw/char/pl011.c | 105 +---
> include/hw/char/pl011.h | 3 ++
> 2 files changed, 102 insertions(+), 6 deletions(-)
Thanks for this patch. I have some comments on some bits of the
code below.
> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> index 13e784f9d9..dccb8c42b0 100644
> --- a/hw/char/pl011.c
> +++ b/hw/char/pl011.c
> @@ -169,6 +169,73 @@ static void pl011_set_read_trigger(PL011State *s)
> s->read_trigger = 1;
> }
>
> +static gboolean pl011_xmit(GIOChannel *chan, GIOCondition cond, void *opaque)
> +{
> +PL011State *s = PL011(opaque);
> +int ret;
> +
> +/* Drain FIFO if there is no backend */
> +if (!qemu_chr_fe_backend_connected(&s->chr)) {
> +s->write_count = 0;
> +s->flags &= ~PL011_FLAG_TXFF;
> +s->flags |= PL011_FLAG_TXFE;
> +return G_SOURCE_REMOVE;
> +}
This "handle no backend" code isn't necessary. There was a
period of time when it was, because some of the qemu_chr_fe_*
functions did the wrong thing if called on a CharBackend with
a NULL Chardev, which is why some code in the tree checks it,
but we fixed that. If there's no backend then both
qemu_chr_fe_write() and qemu_chr_fe_add_watch() will return 0
without doing anything, which will make us drain the FIFO
via the "!s->watch_tag" code path below.
> +
> +/* Nothing to do */
> +if (!s->write_count) {
> +return FALSE;
> +}
> +
> +ret = qemu_chr_fe_write(&s->chr, s->write_fifo, s->write_count);
> +if (ret > 0) {
> +s->write_count -= ret;
> +memmove(s->write_fifo, s->write_fifo + ret, s->write_count);
> +s->flags &= ~PL011_FLAG_TXFF;
> +if (!s->write_count) {
> +s->flags |= PL011_FLAG_TXFE;
> +}
> +}
> +
> +if (s->write_count) {
> +s->watch_tag = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
> + pl011_xmit, s);
> +if (!s->watch_tag) {
> +s->write_count = 0;
> +s->flags &= ~PL011_FLAG_TXFF;
> +s->flags |= PL011_FLAG_TXFE;
> +return FALSE;
> +}
> +}
> +
> +s->int_level |= PL011_INT_TX;
Handling of INT_TX is more complicated when the FIFO is
enabled: the UARTIFLS.TXIFLSEL bits define at what point
we should raise the TX interrupt as the FIFO drains
(eg you can make it interrupt as the FIFO passes through
the "half full" point, or when it gets <= 1/8th full, etc).
Watch out that the definition is that the interrupt is raised
as the FIFO fill level progresses through the trigger
level, which is not the same as "is the FIFO fill level
less than or equal to the trigger level now?".
> +pl011_update(s);
> +return FALSE;
> +}
> +
> +static void pl011_write_fifo(void *opaque, const unsigned char *buf, int
> size)
> +{
> +PL011State *s = PL011(opaque);
> +int depth = (s->lcr & 0x10) ? 16 : 1;
> +
> +if (size >= depth - s->write_count) {
> +size = depth - s->write_count;
> +}
> +
> +if (size > 0) {
> +memcpy(s->write_fifo + s->write_count, buf, size);
> +s->write_count += size;
> +if (s->write_count >= depth) {
> +s->flags |= PL011_FLAG_TXFF;
> +}
> +s->flags &= ~PL011_FLAG_TXFE;
> +}
> +
> +if (!s->watch_tag) {
> +pl011_xmit(NULL, G_IO_OUT, s);
> +}
> +}
It looks like we only ever call pl011_write_fifo() with
a size of 1 -- should we just make it directly take
a single 'uint8_t ch' to write to the FIFO? It would
simplify some of this code I think.
The UARTFR.BUSY bit should be set to 1 as soon as the UART
gets data into the tx FIFO and then cleared only when the
data has all been transmitted. We didn't need to worry about
that when we blocked until the data was sent (the guest could
not execute at a point where it would see BUSY=1), but now
we model the tx FIFO we need to update the BUSY bit (both
in this function to set it and then in anywhere that
empties the FIFO to clear it).
> +
> static void pl011_write(void *opaque, hwaddr offset,
> uint64_t value, unsigned size)
> {
> @@ -179,13 +246,8 @@ static void pl011_write(void *opaque
[PATCH] target/arm: Use correct variable for setting 'max' cpu's ID_AA64DFR0
In aarch64_max_initfn() we update both 32-bit and 64-bit ID registers. The intended pattern is that for 64-bit ID registers we use FIELD_DP64 and the uint64_t 't' register, while 32-bit ID registers use FIELD_DP32 and the uint32_t 'u' register. For ID_AA64DFR0 we accidentally used 'u', meaning that the top 32 bits of this 64-bit ID register would end up always zero. Luckily at the moment that's what they should be anyway, so this bug has no visible effects. Use the right-sized variable. Fixes: 3bec78447a958d481991 Signed-off-by: Peter Maydell --- target/arm/cpu64.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 95d0c8c101a..4c7105ea1a1 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -708,9 +708,9 @@ static void aarch64_max_initfn(Object *obj) u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */ cpu->isar.id_mmfr4 = u; -u = cpu->isar.id_aa64dfr0; -u = FIELD_DP64(u, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */ -cpu->isar.id_aa64dfr0 = u; +t = cpu->isar.id_aa64dfr0; +t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */ +cpu->isar.id_aa64dfr0 = t; u = cpu->isar.id_dfr0; u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */ -- 2.20.1
Re: [PATCH v5 7/9] block: truncate: Don't make backing file data visible
On 22.04.20 17:21, Kevin Wolf wrote:
> When extending the size of an image that has a backing file larger than
> its old size, make sure that the backing file data doesn't become
> visible in the guest, but the added area is properly zeroed out.
>
> Consider the following scenario where the overlay is shorter than its
> backing file:
>
> base.qcow2:
> overlay.qcow2:
>
> When resizing (extending) overlay.qcow2, the new blocks should not stay
> unallocated and make the additional As from base.qcow2 visible like
> before this patch, but zeros should be read.
>
> A similar case happens with the various variants of a commit job when an
> intermediate file is short (- for unallocated):
>
> base.qcow2: A-A-
> mid.qcow2: BB-B
> top.qcow2: C--C--C-
>
> After commit top.qcow2 to mid.qcow2, the following happens:
>
> mid.qcow2: CB-C00C0 (correct result)
> mid.qcow2: CB-C--C- (before this fix)
>
> Without the fix, blocks that previously read as zeros on top.qcow2
> suddenly turn into A.
>
> Signed-off-by: Kevin Wolf
> Reviewed-by: Alberto Garcia
> ---
> block/io.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/block/io.c b/block/io.c
> index 795075954e..8fbb607515 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -3394,6 +3394,20 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
> int64_t offset, bool exact,
> goto out;
> }
>
> +/*
> + * If the image has a backing file that is large enough that it would
> + * provide data for the new area, we cannot leave it unallocated because
> + * then the backing file content would become visible. Instead, zero-fill
> + * the new area.
> + *
> + * Note that if the image has a backing file, but was opened without the
> + * backing file, taking care of keeping things consistent with that
> backing
> + * file is the user's responsibility.
> + */
> +if (new_bytes && bs->backing) {
> +flags |= BDRV_REQ_ZERO_WRITE;
> +}
This breaks growing any non-qcow2 image with any backing file. Do we
care about that?
The comment says something about “a backing file that is large enough
that it would provide data for the new area”, but that condition doesn’t
appear in the code. Should it? (If it did, I think the number of cases
this change broke would be much smaller.)
If it was deliberate to not have that condition here, and if we decide
that we don’t care about non-qcow2 formats here, then I think at least
the error message deserves some improvement over “qemu-img: Block driver
does not support requested flags”.
Max
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v1 3/3] hw/arm: xlnx-zcu102: Disable unsupported FDT firmware nodes
On Sun, 19 Apr 2020 at 17:27, Edgar E. Iglesias
wrote:
>
> From: "Edgar E. Iglesias"
>
> Disable unsupported FDT firmware nodes if a user passes us
> a DTB with nodes enabled that the machine cannot support
> due to lack of EL3 or EL2 support.
>
> Signed-off-by: Edgar E. Iglesias
> ---
> hw/arm/xlnx-zcu102.c | 31 +++
> 1 file changed, 31 insertions(+)
> +static void zcu102_modify_dtb(const struct arm_boot_info *binfo, void *fdt)
> +{
> +XlnxZCU102 *s = container_of(binfo, XlnxZCU102, binfo);
> +bool method_is_hvc;
> +char **node_path;
> +const char *r;
> +int prop_len;
> +int i;
> +
> +/* If EL3 is enabled, we keep all firmware nodes active. */
> +if (!s->secure) {
> +node_path = qemu_fdt_node_path(fdt, NULL,
> + (char *)"xlnx,zynqmp-firmware",
> + &error_fatal);
Why do we need the 'char *' cast ?
thanks
-- PMM
Re: [PATCH v5 8/9] iotests: Filter testfiles out in filter_img_info()
On 22.04.20 17:21, Kevin Wolf wrote: > We want to keep TEST_IMG for the full path of the main test image, but > filter_testfiles() must be called for other test images before replacing > other things like the image format because the test directory path could > contain the format as a substring. > > Insert a filter_testfiles() call between both. > > Signed-off-by: Kevin Wolf > --- > tests/qemu-iotests/iotests.py | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's ID_AA64DFR0
On Thu, Apr 23, 2020 at 1:09 PM Peter Maydell wrote: > > In aarch64_max_initfn() we update both 32-bit and 64-bit ID > registers. The intended pattern is that for 64-bit ID registers we > use FIELD_DP64 and the uint64_t 't' register, while 32-bit ID > registers use FIELD_DP32 and the uint32_t 'u' register. For > ID_AA64DFR0 we accidentally used 'u', meaning that the top 32 bits of > this 64-bit ID register would end up always zero. Luckily at the > moment that's what they should be anyway, so this bug has no visible > effects. > > Use the right-sized variable. > > Fixes: 3bec78447a958d481991 > Signed-off-by: Peter Maydell Reviewed-by: Laurent Desnogues Thanks, Laurent > --- > target/arm/cpu64.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 95d0c8c101a..4c7105ea1a1 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -708,9 +708,9 @@ static void aarch64_max_initfn(Object *obj) > u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */ > cpu->isar.id_mmfr4 = u; > > -u = cpu->isar.id_aa64dfr0; > -u = FIELD_DP64(u, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */ > -cpu->isar.id_aa64dfr0 = u; > +t = cpu->isar.id_aa64dfr0; > +t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */ > +cpu->isar.id_aa64dfr0 = t; > > u = cpu->isar.id_dfr0; > u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */ > -- > 2.20.1 > >
Re: [PATCH] virtio-vga: fix virtio-vga bar ordering
Hi, > Just a question, why didn't we choose the virtio-vga order to avoid > shuffling from the beginning? Vga came after and we keep the > compatibility ? Well, transitional virtio devices need bar 0 for legacy virtio compatibility (io bar), so using bar 1 for msix makes sense in that case. virtio-vga is new enough that it supports modern only so it doesn't need to worry about legacy virtio compatibility. It does have to worry about vga compatibility though. Therefore it uses a bar layout different from anyone else ... cheers, Gerd
Re: [PATCH v2 07/14] bochs-display: Fix vgamem=SIZE error handling
On Wed, Apr 22, 2020 at 03:07:12PM +0200, Markus Armbruster wrote: > bochs_display_realize() rejects out-of-range vgamem. The error > handling is broken: > > $ qemu-system-x86_64 -S -display none -monitor stdio > QEMU 4.2.93 monitor - type 'help' for more information > (qemu) device_add bochs-display,vgamem=1 > Error: bochs-display: video memory too small > (qemu) device_add bochs-display,vgamem=1 > RAMBlock ":00:04.0/bochs-display-vram" already registered, abort! > Aborted (core dumped) > > Cause: bochs_display_realize() neglects to bail out after setting the > error. Fix that. > > Fixes: 765c94290863eef1fc4a67819d452cc13b7854a1 > Cc: Gerd Hoffmann > Signed-off-by: Markus Armbruster > Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Gerd Hoffmann
Re: [PATCH v2 1/2] virtio-vga: fix virtio-vga bar ordering
On Wed, Apr 22, 2020 at 11:54:54PM +0200, Anthoine Bourgeois wrote:
> With virtio-vga, pci bar are reordered. Bar #2 is used for compatibility
> with stdvga. By default, bar #2 is used by virtio modern io bar.
> This bar is the last one introduce in the virtio pci bar layout and it's
> crushed by the virtio-vga reordering. So virtio-vga and
> modern-pio-notify are incompatible because virtio-vga failed to
> initialize with this option.
>
> This fix sets the modern io bar to the bar #5 to avoid conflict.
>
> Signed-off-by: Anthoine Bourgeois
Reviewed-by: Gerd Hoffmann
> ---
> hw/display/virtio-vga.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
> index 2b4c2aa126..95757a6619 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -114,6 +114,7 @@ static void virtio_vga_base_realize(VirtIOPCIProxy
> *vpci_dev, Error **errp)
> */
> vpci_dev->modern_mem_bar_idx = 2;
> vpci_dev->msix_bar_idx = 4;
> +vpci_dev->modern_io_bar_idx = 5;
>
> if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
> /*
> --
> 2.20.1
>
Re: [PATCH v2 2/2] virtio-pci: update virtio pci bar layout documentation
On Wed, Apr 22, 2020 at 11:54:55PM +0200, Anthoine Bourgeois wrote: > The modern io bar was never documented. > > Signed-off-by: Anthoine Bourgeois > --- > hw/virtio/virtio-pci.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c > index 4cb784389c..d028c17c24 100644 > --- a/hw/virtio/virtio-pci.c > +++ b/hw/virtio/virtio-pci.c > @@ -1705,6 +1705,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, > Error **errp) > * > * region 0 -- virtio legacy io bar > * region 1 -- msi-x bar > + * region 2 -- virtio modern io bar (off by default) > * region 4+5 -- virtio modern memory (64bit) bar Reviewed-by: Gerd Hoffmann > * > */ > -- > 2.20.1 >
Re: [PATCH] target/arm: Vectorize integer comparison vs zero
On Sat, 18 Apr 2020 at 17:28, Richard Henderson wrote: > > These instructions are often used in glibc's string routines. > They were the final uses of the 32-bit at a time neon helpers. > > Signed-off-by: Richard Henderson Applied to target-arm.next, thanks. Luckily my decodetree conversion for Neon had not yet got to any of the 2-reg insns :-) -- PMM
Re: [PATCH 4/5] ramfb: add sanity checks to ramfb_create_display_surface
Hi,
> - if "linesize" is nonzero, make sure it is a whole multiple of the
> required word size (?)
>
> - if "linesize" is nonzero, make sure it is not bogus with relation to
> "width". I'm thinking something like:
Yep, the whole linesize is a bit bogus, noticed after sending out this
series, I have a followup patch for that (see below).
take care,
Gerd
>From 154dcff73dc533fc95c88bd960ed65108af6c734 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann
Date: Wed, 22 Apr 2020 12:07:59 +0200
Subject: [PATCH] ramfb: fix size calculation
size calculation isn't correct with guest-supplied stride, the last
display line isn't accounted for correctly.
For the typical case of stride > linesize (add padding) we error on the
safe side (calculated size is larger than actual size).
With stride < linesize (scanlines overlap) the calculated size is
smaller than the actual size though so our guest memory mapping might
end up being too small.
While being at it also fix ramfb_create_display_surface to use hwaddr
for the parameters. That way all calculation are done with hwaddr type
and we can't get funny effects from type castings.
Signed-off-by: Gerd Hoffmann
---
hw/display/ramfb.c | 19 ++-
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/display/ramfb.c b/hw/display/ramfb.c
index be884c9ea837..928d74d10bc7 100644
--- a/hw/display/ramfb.c
+++ b/hw/display/ramfb.c
@@ -44,10 +44,10 @@ static void ramfb_unmap_display_surface(pixman_image_t
*image, void *unused)
static DisplaySurface *ramfb_create_display_surface(int width, int height,
pixman_format_code_t
format,
-int linesize, uint64_t
addr)
+hwaddr stride, hwaddr addr)
{
DisplaySurface *surface;
-hwaddr size;
+hwaddr size, mapsize, linesize;
void *data;
if (width < 16 || width > VBE_DISPI_MAX_XRES ||
@@ -55,19 +55,20 @@ static DisplaySurface *ramfb_create_display_surface(int
width, int height,
format == 0 /* unknown format */)
return NULL;
-if (linesize == 0) {
-linesize = width * PIXMAN_FORMAT_BPP(format) / 8;
+linesize = width * PIXMAN_FORMAT_BPP(format) / 8;
+if (stride == 0) {
+stride = linesize;
}
-size = (hwaddr)linesize * height;
-data = cpu_physical_memory_map(addr, &size, false);
-if (size != (hwaddr)linesize * height) {
-cpu_physical_memory_unmap(data, size, 0, 0);
+mapsize = size = stride * (height - 1) + linesize;
+data = cpu_physical_memory_map(addr, &mapsize, false);
+if (size != mapsize) {
+cpu_physical_memory_unmap(data, mapsize, 0, 0);
return NULL;
}
surface = qemu_create_displaysurface_from(width, height,
- format, linesize, data);
+ format, stride, data);
pixman_image_set_destroy_function(surface->image,
ramfb_unmap_display_surface, NULL);
--
2.18.2
Re: [PATCH v1 3/3] hw/arm: xlnx-zcu102: Disable unsupported FDT firmware nodes
On Thu, Apr 23, 2020 at 12:21:11PM +0100, Peter Maydell wrote:
> On Sun, 19 Apr 2020 at 17:27, Edgar E. Iglesias
> wrote:
> >
> > From: "Edgar E. Iglesias"
> >
> > Disable unsupported FDT firmware nodes if a user passes us
> > a DTB with nodes enabled that the machine cannot support
> > due to lack of EL3 or EL2 support.
> >
> > Signed-off-by: Edgar E. Iglesias
> > ---
> > hw/arm/xlnx-zcu102.c | 31 +++
> > 1 file changed, 31 insertions(+)
> > +static void zcu102_modify_dtb(const struct arm_boot_info *binfo, void *fdt)
> > +{
> > +XlnxZCU102 *s = container_of(binfo, XlnxZCU102, binfo);
> > +bool method_is_hvc;
> > +char **node_path;
> > +const char *r;
> > +int prop_len;
> > +int i;
> > +
> > +/* If EL3 is enabled, we keep all firmware nodes active. */
> > +if (!s->secure) {
> > +node_path = qemu_fdt_node_path(fdt, NULL,
> > + (char *)"xlnx,zynqmp-firmware",
> > + &error_fatal);
>
> Why do we need the 'char *' cast ?
Without it, I see the following warning but compat in
qemu_fdt_node_path should probably be changed to const char *.
I can make that change in a v2 if you prefer.
CC aarch64-softmmu/hw/arm/xlnx-zcu102.o
/home/edgar/src/c/qemu/qemu/hw/arm/xlnx-zcu102.c: In function
‘zcu102_modify_dtb’:
/home/edgar/src/c/qemu/qemu/hw/arm/xlnx-zcu102.c:84:40: error: passing argument
3 of ‘qemu_fdt_node_path’ discards ‘const’ qualifier from pointer target type
[-Werror=discarded-qualifiers]
84 |"xlnx,zynqmp-firmware",
|^~
In file included from /home/edgar/src/c/qemu/qemu/hw/arm/xlnx-zcu102.c:26:
/home/edgar/src/c/qemu/qemu/include/sysemu/device_tree.h:46:8: note: expected
‘char *’ but argument is of type ‘const char *’
46 | char **qemu_fdt_node_path(void *fdt, const char *name, char *compat,
|^~
cc1: all warnings being treated as errors
make[1]: *** [/home/edgar/src/c/qemu/qemu/rules.mak:69: hw/arm/xlnx-zcu102.o]
Error 1
make: *** [Makefile:527: aarch64-softmmu/all] Error 2
Cheers,
Edgar
Re: [PATCH v1 3/3] hw/arm: xlnx-zcu102: Disable unsupported FDT firmware nodes
On Thu, 23 Apr 2020 at 12:43, Edgar E. Iglesias wrote: > Without it, I see the following warning but compat in > qemu_fdt_node_path should probably be changed to const char *. > I can make that change in a v2 if you prefer. Yes, I think that would be better. I can't see any reason why the compat argument needs to be non-const. thanks -- PMM
Re: [PATCH v5 9/9] iotests: Test committing to short backing file
On 22.04.20 17:21, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf > --- > tests/qemu-iotests/274 | 157 ++ > tests/qemu-iotests/274.out | 260 + > tests/qemu-iotests/group | 1 + > 3 files changed, 418 insertions(+) > create mode 100755 tests/qemu-iotests/274 > create mode 100644 tests/qemu-iotests/274.out Reviewed-by: Max Reitz signature.asc Description: OpenPGP digital signature
Re: [PATCH v2 0/2] hw/arm/virt: dt: add kaslr-seed property
On Mon, 20 Apr 2020 at 13:20, Jerome Forissier wrote: > > This patchset creates the DT property /chosen/kaslr-seed which is used > by the OS for Address Space Layout Randomization. If the machine is > secure, a similar property is created under /secure-chosen. > > Changes since v1: > - Move creation of /secure-chosen to create_fdt() > - Use qemu_guest_getrandom() instead of qcrypto_random_bytes() > - Create kaslr-seed for the non-secure OS too > > Jerome Forissier (2): > hw/arm/virt: dt: move creation of /secure-chosen to create_fdt() > hw/arm/virt: dt: add kaslr-seed property Applied to target-arm.next, thanks. -- PMM
[PATCH v2 0/4] hw/arm: xlnx-zcu102: Disable unsupported FDT firmware nodes
From: "Edgar E. Iglesias" When users try direct Linux runs on the ZynqMP models without enabling EL3 (and using appropriate FW) they run into trouble because the upstream kernel device-tree has EL3 based firmware nodes by default. PSCI firmware nodes work because we emulate the firmware in QEMU. This series avoids that problem by disabling firmware nodes that the machine cannot support due to lack of EL3 or EL2 support. This means we can now (without manually editing DTBs) run the following in a current Linux tree: qemu-system-aarch64 -M xlnx-zcu102 -m 2G -dtb arch/arm64/boot/dts/xilinx/zynqmp-zcu102-rev1.0.dtb -serial mon:stdio -kernel arch/arm64/boot/Image -initrd zu-rootfs.cpio.gz -append rdinit=/bin/sh Cheers, Edgar ChangeLog: v1 -> v2: * Constify compat in qemu_fdt_node_path(). Edgar E. Iglesias (4): device_tree: Allow name wildcards in qemu_fdt_node_path() device_tree: Constify compat in qemu_fdt_node_path() hw/arm: xlnx-zcu102: Move arm_boot_info into XlnxZCU102 hw/arm: xlnx-zcu102: Disable unsupported FDT firmware nodes device_tree.c| 4 ++-- hw/arm/xlnx-zcu102.c | 39 include/sysemu/device_tree.h | 5 - 3 files changed, 41 insertions(+), 7 deletions(-) -- 2.20.1
[PATCH v2 1/4] device_tree: Allow name wildcards in qemu_fdt_node_path()
From: "Edgar E. Iglesias"
Allow name wildcards in qemu_fdt_node_path(). This is useful
to find all nodes with a given compatibility string.
Reviewed-by: Alistair Francis
Signed-off-by: Edgar E. Iglesias
---
device_tree.c| 2 +-
include/sysemu/device_tree.h | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/device_tree.c b/device_tree.c
index bba6cc2164..f5b4699aed 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -308,7 +308,7 @@ char **qemu_fdt_node_path(void *fdt, const char *name, char
*compat,
offset = len;
break;
}
-if (!strcmp(iter_name, name)) {
+if (!name || !strcmp(iter_name, name)) {
char *path;
path = g_malloc(path_len);
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index c16fd69bc0..7c53ef7634 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -39,6 +39,9 @@ void *load_device_tree_from_sysfs(void);
* NULL. If there is no error but no matching node was found, the
* returned array contains a single element equal to NULL. If an error
* was encountered when parsing the blob, the function returns NULL
+ *
+ * @name may be NULL to wildcard names and only match compatibility
+ * strings.
*/
char **qemu_fdt_node_path(void *fdt, const char *name, char *compat,
Error **errp);
--
2.20.1
[PATCH v2 2/4] device_tree: Constify compat in qemu_fdt_node_path()
From: "Edgar E. Iglesias"
Make compat in qemu_fdt_node_path() const char *.
Signed-off-by: Edgar E. Iglesias
---
device_tree.c| 2 +-
include/sysemu/device_tree.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/device_tree.c b/device_tree.c
index f5b4699aed..b335dae707 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -291,7 +291,7 @@ char **qemu_fdt_node_unit_path(void *fdt, const char *name,
Error **errp)
return path_array;
}
-char **qemu_fdt_node_path(void *fdt, const char *name, char *compat,
+char **qemu_fdt_node_path(void *fdt, const char *name, const char *compat,
Error **errp)
{
int offset, len, ret;
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 7c53ef7634..982c89345f 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -43,7 +43,7 @@ void *load_device_tree_from_sysfs(void);
* @name may be NULL to wildcard names and only match compatibility
* strings.
*/
-char **qemu_fdt_node_path(void *fdt, const char *name, char *compat,
+char **qemu_fdt_node_path(void *fdt, const char *name, const char *compat,
Error **errp);
/**
--
2.20.1
[PATCH v2 3/4] hw/arm: xlnx-zcu102: Move arm_boot_info into XlnxZCU102
From: "Edgar E. Iglesias"
Move arm_boot_info into XlnxZCU102.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Edgar E. Iglesias
---
hw/arm/xlnx-zcu102.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index bd645ad818..4eb117c755 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -31,13 +31,14 @@ typedef struct XlnxZCU102 {
bool secure;
bool virt;
+
+struct arm_boot_info binfo;
} XlnxZCU102;
#define TYPE_ZCU102_MACHINE MACHINE_TYPE_NAME("xlnx-zcu102")
#define ZCU102_MACHINE(obj) \
OBJECT_CHECK(XlnxZCU102, (obj), TYPE_ZCU102_MACHINE)
-static struct arm_boot_info xlnx_zcu102_binfo;
static bool zcu102_get_secure(Object *obj, Error **errp)
{
@@ -166,9 +167,9 @@ static void xlnx_zcu102_init(MachineState *machine)
/* TODO create and connect IDE devices for ide_drive_get() */
-xlnx_zcu102_binfo.ram_size = ram_size;
-xlnx_zcu102_binfo.loader_start = 0;
-arm_load_kernel(s->soc.boot_cpu_ptr, machine, &xlnx_zcu102_binfo);
+s->binfo.ram_size = ram_size;
+s->binfo.loader_start = 0;
+arm_load_kernel(s->soc.boot_cpu_ptr, machine, &s->binfo);
}
static void xlnx_zcu102_machine_instance_init(Object *obj)
--
2.20.1
[PATCH v2 4/4] hw/arm: xlnx-zcu102: Disable unsupported FDT firmware nodes
From: "Edgar E. Iglesias"
Disable unsupported FDT firmware nodes if a user passes us
a DTB with nodes enabled that the machine cannot support
due to lack of EL3 or EL2 support.
Reviewed-by: Alistair Francis
Signed-off-by: Edgar E. Iglesias
---
hw/arm/xlnx-zcu102.c | 30 ++
1 file changed, 30 insertions(+)
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 4eb117c755..a798e228b7 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -23,6 +23,7 @@
#include "qemu/error-report.h"
#include "qemu/log.h"
#include "sysemu/qtest.h"
+#include "sysemu/device_tree.h"
typedef struct XlnxZCU102 {
MachineState parent_obj;
@@ -68,6 +69,34 @@ static void zcu102_set_virt(Object *obj, bool value, Error
**errp)
s->virt = value;
}
+static void zcu102_modify_dtb(const struct arm_boot_info *binfo, void *fdt)
+{
+XlnxZCU102 *s = container_of(binfo, XlnxZCU102, binfo);
+bool method_is_hvc;
+char **node_path;
+const char *r;
+int prop_len;
+int i;
+
+/* If EL3 is enabled, we keep all firmware nodes active. */
+if (!s->secure) {
+node_path = qemu_fdt_node_path(fdt, NULL, "xlnx,zynqmp-firmware",
+ &error_fatal);
+
+for (i = 0; node_path && node_path[i]; i++) {
+r = qemu_fdt_getprop(fdt, node_path[i], "method", &prop_len, NULL);
+method_is_hvc = r && !strcmp("hvc", r);
+
+/* Allow HVC based firmware if EL2 is enabled. */
+if (method_is_hvc && s->virt) {
+continue;
+}
+qemu_fdt_setprop_string(fdt, node_path[i], "status", "disabled");
+}
+g_strfreev(node_path);
+}
+}
+
static void xlnx_zcu102_init(MachineState *machine)
{
XlnxZCU102 *s = ZCU102_MACHINE(machine);
@@ -169,6 +198,7 @@ static void xlnx_zcu102_init(MachineState *machine)
s->binfo.ram_size = ram_size;
s->binfo.loader_start = 0;
+s->binfo.modify_dtb = zcu102_modify_dtb;
arm_load_kernel(s->soc.boot_cpu_ptr, machine, &s->binfo);
}
--
2.20.1
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's ID_AA64DFR0
On Thu, Apr 23, 2020 at 1:09 PM Peter Maydell wrote:
>
> In aarch64_max_initfn() we update both 32-bit and 64-bit ID
> registers. The intended pattern is that for 64-bit ID registers we
> use FIELD_DP64 and the uint64_t 't' register, while 32-bit ID
> registers use FIELD_DP32 and the uint32_t 'u' register.
Variable names could be improved...
> For
> ID_AA64DFR0 we accidentally used 'u', meaning that the top 32 bits of
> this 64-bit ID register would end up always zero.
-Wconversion CPPFLAG helps but there are so many places to fix that we
are not using it:
target/arm/cpu64.c:711:13: error: conversion from ‘uint64_t’ {aka
‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change
value [-Werror=conversion]
711 | u = cpu->isar.id_aa64dfr0;
| ^~~
target/arm/cpu64.c:712:13: note: in expansion of macro ‘FIELD_DP64’
712 | u = FIELD_DP64(u, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
| ^~
> Luckily at the
> moment that's what they should be anyway, so this bug has no visible
> effects.
>
> Use the right-sized variable.
>
> Fixes: 3bec78447a958d481991
> Signed-off-by: Peter Maydell
Reviewed-by: Philippe Mathieu-Daudé
> ---
> target/arm/cpu64.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 95d0c8c101a..4c7105ea1a1 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -708,9 +708,9 @@ static void aarch64_max_initfn(Object *obj)
> u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */
> cpu->isar.id_mmfr4 = u;
>
> -u = cpu->isar.id_aa64dfr0;
> -u = FIELD_DP64(u, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
> -cpu->isar.id_aa64dfr0 = u;
> +t = cpu->isar.id_aa64dfr0;
> +t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
> +cpu->isar.id_aa64dfr0 = t;
>
> u = cpu->isar.id_dfr0;
> u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */
> --
> 2.20.1
>
>
Re: [PATCH v5 1/9] block: Add flags to BlockDriver.bdrv_co_truncate()
Am 23.04.2020 um 11:41 hat Max Reitz geschrieben: > On 22.04.20 17:21, Kevin Wolf wrote: > > This adds a new BdrvRequestFlags parameter to the .bdrv_co_truncate() > > driver callbacks, and a supported_truncate_flags field in > > BlockDriverState that allows drivers to advertise support for request > > flags in the context of truncate. > > > > For now, we always pass 0 and no drivers declare support for any flag. > > > > Signed-off-by: Kevin Wolf > > Reviewed-by: Vladimir Sementsov-Ogievskiy > > Reviewed-by: Alberto Garcia > > --- > > include/block/block_int.h | 10 +- > > block/crypto.c | 3 ++- > > block/file-posix.c | 2 +- > > block/file-win32.c | 2 +- > > block/gluster.c | 1 + > > block/io.c | 8 +++- > > block/iscsi.c | 2 +- > > block/nfs.c | 3 ++- > > block/qcow2.c | 2 +- > > block/qed.c | 1 + > > block/raw-format.c | 2 +- > > block/rbd.c | 1 + > > block/sheepdog.c| 4 ++-- > > block/ssh.c | 2 +- > > tests/test-block-iothread.c | 3 ++- > > 15 files changed, 33 insertions(+), 13 deletions(-) > > (I know I haven’t complained before, so *shrug*, but I wonder now > whether it actually makes sense to have the same BdrvRequestFlags for > all request types. Or why we have the same flags type for read, write, > and zero-write already.) Yeah, nothing this series introduces. I wonder, too, but as long as we have enough bits to cover flags for all request types, and because we have overlaps between the request types, it might be easier to have only one set of flags. So it might be accidental, but I actually feel the current state isn't bad. Kevin signature.asc Description: PGP signature
[PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
MIDR_EL1 is a 32-bit register.
This fixes when compiling with -Werror=conversion:
target/arm/cpu64.c: In function ‘aarch64_max_initfn’:
target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long
unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value
[-Werror=conversion]
628 | cpu->midr = t;
| ^
Signed-off-by: Philippe Mathieu-Daudé
---
target/arm/cpu64.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 95d0c8c101..4eb0a9030e 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -620,12 +620,12 @@ static void aarch64_max_initfn(Object *obj)
* code needs to distinguish this QEMU CPU from other software
* implementations, though this shouldn't be needed.
*/
-t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
-t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
-t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
-t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
-t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
-cpu->midr = t;
+u = FIELD_DP32(0, MIDR_EL1, IMPLEMENTER, 0);
+u = FIELD_DP32(u, MIDR_EL1, ARCHITECTURE, 0xf);
+u = FIELD_DP32(u, MIDR_EL1, PARTNUM, 'Q');
+u = FIELD_DP32(u, MIDR_EL1, VARIANT, 0);
+u = FIELD_DP32(u, MIDR_EL1, REVISION, 0);
+cpu->midr = u;
t = cpu->isar.id_aa64isar0;
t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
--
2.21.1
Re: [PATCH v2 1/2] virtio-vga: fix virtio-vga bar ordering
On Wed, Apr 22, 2020 at 11:54:54PM +0200, Anthoine Bourgeois wrote:
> With virtio-vga, pci bar are reordered. Bar #2 is used for compatibility
> with stdvga. By default, bar #2 is used by virtio modern io bar.
> This bar is the last one introduce in the virtio pci bar layout and it's
> crushed by the virtio-vga reordering. So virtio-vga and
> modern-pio-notify are incompatible because virtio-vga failed to
> initialize with this option.
>
> This fix sets the modern io bar to the bar #5 to avoid conflict.
>
> Signed-off-by: Anthoine Bourgeois
Gerd, would you say it's required for 5.0?
> ---
> hw/display/virtio-vga.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
> index 2b4c2aa126..95757a6619 100644
> --- a/hw/display/virtio-vga.c
> +++ b/hw/display/virtio-vga.c
> @@ -114,6 +114,7 @@ static void virtio_vga_base_realize(VirtIOPCIProxy
> *vpci_dev, Error **errp)
> */
> vpci_dev->modern_mem_bar_idx = 2;
> vpci_dev->msix_bar_idx = 4;
> +vpci_dev->modern_io_bar_idx = 5;
>
> if (!(vpci_dev->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)) {
> /*
> --
> 2.20.1
Re: [PATCH v5 7/9] block: truncate: Don't make backing file data visible
Am 23.04.2020 um 13:14 hat Max Reitz geschrieben:
> On 22.04.20 17:21, Kevin Wolf wrote:
> > When extending the size of an image that has a backing file larger than
> > its old size, make sure that the backing file data doesn't become
> > visible in the guest, but the added area is properly zeroed out.
> >
> > Consider the following scenario where the overlay is shorter than its
> > backing file:
> >
> > base.qcow2:
> > overlay.qcow2:
> >
> > When resizing (extending) overlay.qcow2, the new blocks should not stay
> > unallocated and make the additional As from base.qcow2 visible like
> > before this patch, but zeros should be read.
> >
> > A similar case happens with the various variants of a commit job when an
> > intermediate file is short (- for unallocated):
> >
> > base.qcow2: A-A-
> > mid.qcow2: BB-B
> > top.qcow2: C--C--C-
> >
> > After commit top.qcow2 to mid.qcow2, the following happens:
> >
> > mid.qcow2: CB-C00C0 (correct result)
> > mid.qcow2: CB-C--C- (before this fix)
> >
> > Without the fix, blocks that previously read as zeros on top.qcow2
> > suddenly turn into A.
> >
> > Signed-off-by: Kevin Wolf
> > Reviewed-by: Alberto Garcia
> > ---
> > block/io.c | 14 ++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/block/io.c b/block/io.c
> > index 795075954e..8fbb607515 100644
> > --- a/block/io.c
> > +++ b/block/io.c
> > @@ -3394,6 +3394,20 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child,
> > int64_t offset, bool exact,
> > goto out;
> > }
> >
> > +/*
> > + * If the image has a backing file that is large enough that it would
> > + * provide data for the new area, we cannot leave it unallocated
> > because
> > + * then the backing file content would become visible. Instead,
> > zero-fill
> > + * the new area.
> > + *
> > + * Note that if the image has a backing file, but was opened without
> > the
> > + * backing file, taking care of keeping things consistent with that
> > backing
> > + * file is the user's responsibility.
> > + */
> > +if (new_bytes && bs->backing) {
> > +flags |= BDRV_REQ_ZERO_WRITE;
> > +}
>
> This breaks growing any non-qcow2 image with any backing file. Do we
> care about that?
>
> The comment says something about “a backing file that is large enough
> that it would provide data for the new area”, but that condition doesn’t
> appear in the code. Should it? (If it did, I think the number of cases
> this change broke would be much smaller.)
>
> If it was deliberate to not have that condition here, and if we decide
> that we don’t care about non-qcow2 formats here, then I think at least
> the error message deserves some improvement over “qemu-img: Block driver
> does not support requested flags”.
This was not deliberate. v3 had the check and I'm not sure why I removed
it. Probably because the new approach felt so much simpler and I was
glad that I could throw away complicated code that I threw away more
than I should have...
Kevin
signature.asc
Description: PGP signature
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
On Thu, Apr 23, 2020 at 2:44 PM Philippe Mathieu-Daudé wrote:
>
> MIDR_EL1 is a 32-bit register.
In fact MIDR_EL1 a 64-bit system register with the top 32-bit being RES0.
So the right fix might be to change midr field size, just to be future proof :-)
But if we stick to a 32-bit midr then:
Reviewed-by: Laurent Desnogues
Thanks,
Laurent
> This fixes when compiling with -Werror=conversion:
>
> target/arm/cpu64.c: In function ‘aarch64_max_initfn’:
> target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long
> unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value
> [-Werror=conversion]
> 628 | cpu->midr = t;
> | ^
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> target/arm/cpu64.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 95d0c8c101..4eb0a9030e 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -620,12 +620,12 @@ static void aarch64_max_initfn(Object *obj)
> * code needs to distinguish this QEMU CPU from other software
> * implementations, though this shouldn't be needed.
> */
> -t = FIELD_DP64(0, MIDR_EL1, IMPLEMENTER, 0);
> -t = FIELD_DP64(t, MIDR_EL1, ARCHITECTURE, 0xf);
> -t = FIELD_DP64(t, MIDR_EL1, PARTNUM, 'Q');
> -t = FIELD_DP64(t, MIDR_EL1, VARIANT, 0);
> -t = FIELD_DP64(t, MIDR_EL1, REVISION, 0);
> -cpu->midr = t;
> +u = FIELD_DP32(0, MIDR_EL1, IMPLEMENTER, 0);
> +u = FIELD_DP32(u, MIDR_EL1, ARCHITECTURE, 0xf);
> +u = FIELD_DP32(u, MIDR_EL1, PARTNUM, 'Q');
> +u = FIELD_DP32(u, MIDR_EL1, VARIANT, 0);
> +u = FIELD_DP32(u, MIDR_EL1, REVISION, 0);
> +cpu->midr = u;
>
> t = cpu->isar.id_aa64isar0;
> t = FIELD_DP64(t, ID_AA64ISAR0, AES, 2); /* AES + PMULL */
> --
> 2.21.1
>
>
Re: [PATCH v5 4/9] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate
Am 22.04.2020 um 18:14 hat Eric Blake geschrieben:
> On 4/22/20 10:58 AM, Kevin Wolf wrote:
>
> > > > @@ -4214,6 +4215,35 @@ static int coroutine_fn
> > > > qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> > > >g_assert_not_reached();
> > > >}
> > > > +if ((flags & BDRV_REQ_ZERO_WRITE) && offset > old_length) {
> > > > +uint64_t zero_start = QEMU_ALIGN_UP(old_length,
> > > > s->cluster_size);
> > > > +uint64_t zero_end = QEMU_ALIGN_UP(offset, s->cluster_size);
> > >
> > > This rounds up beyond the new size...
> > >
> > > > +
> > > > +/* Use zero clusters as much as we can */
> > > > +ret = qcow2_cluster_zeroize(bs, zero_start, zero_end -
> > > > zero_start, 0);
> > >
> > > and then requests that the extra be zeroed. Does that always work, even
> > > when it results in pdrv_co_pwrite_zeroes beyond the end of s->data_file?
> >
> > You mean the data_file_is_raw() path in qcow2_cluster_zeroize()? It's
> > currently not a code path that is run because we only set
> > BDRV_REQ_ZERO_WRITE for truncate if the image has a backing file, and
> > data_file_is_raw() doesn't work with backing files.
>
> Good point.
>
> >
> > But hypothetically, if someone called truncate with BDRV_REQ_ZERO_WRITE
> > for such a file, I think it would fail.
> >
> > > If so,
> > >
> > > Reviewed-by: Eric Blake
> > >
> > > otherwise, you may have to treat the tail specially, the same way you
> > > treated an unaligned head.
> >
> > Actually, do I even need to round the tail?
> >
> > /* Caller must pass aligned values, except at image end */
> > assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
> > assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) ||
> > end_offset == bs->total_sectors << BDRV_SECTOR_BITS);
> >
> > So qcow2_cluster_zeroize() seems to accept the unaligned tail. It would
> > still set the zero flag for the partial last cluster and for the
> > external data file, bdrv_co_pwrite_zeroes() would have the correct size.
>
> Then I'm in favor of NOT rounding the tail. That's an easy enough change
> and we've now justified that it does what we want, so R-b stands with that
> one-line tweak.
Would have been too easy... bs->total_sectors isn't updated yet, so the
assertion does fail.
I can make the assertion check end_offset >= ... instead. That should
still check what we wanted to check here and allow the unaligned
extension.
This feels like the better option to me compared to updating
bs->total_sectors earlier and then undoing that change in every error
path.
Kevin
Re: [PATCH v5 4/9] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate
Am 23.04.2020 um 12:53 hat Max Reitz geschrieben:
> On 22.04.20 17:21, Kevin Wolf wrote:
> > If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling
> > qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't
> > undo any previous preallocation, but just adds the zero flag to all
> > relevant L2 entries. If an external data file is in use, a write_zeroes
> > request to the data file is made instead.
> >
> > Signed-off-by: Kevin Wolf
> > ---
> > block/qcow2.c | 30 ++
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 9cfbdfc939..bd632405d1 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
>
> [...]
>
> > @@ -4214,6 +4215,35 @@ static int coroutine_fn
> > qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> > g_assert_not_reached();
> > }
> >
> > +if ((flags & BDRV_REQ_ZERO_WRITE) && offset > old_length) {
> > +uint64_t zero_start = QEMU_ALIGN_UP(old_length, s->cluster_size);
> > +uint64_t zero_end = QEMU_ALIGN_UP(offset, s->cluster_size);
> > +
> > +/* Use zero clusters as much as we can */
> > +ret = qcow2_cluster_zeroize(bs, zero_start, zero_end - zero_start,
> > 0);
>
> It’s kind of a pity that this changes the cluster mappings that were
> established when using falloc/full preallocation already (i.e., they
> become preallocated zero clusters then, so when writing to them, we need
> COW again).
>
> But falloc/full preallocation do not guarantee that the new data is
> zero, so I suppose this is the only thing we can reasonably do.
If we really want, I guess we could make full preallocation first try
passing BDRV_REQ_ZERO_WRITE to the protocol layer and if that succeeds,
we could skip setting the zero cluster flag at the qcow2 level.
Feels like a separate patch, though.
Kevin
signature.asc
Description: PGP signature
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4...@amsat.org/ Hi, This series failed the asan build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4...@amsat.org/testing.asan/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4...@amsat.org/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1 Message-id: 20200423124305.14718-1-f4...@amsat.org Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 fatal: unable to access 'https://github.com/patchew-project/qemu/': The requested URL returned error: 504 error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384 Traceback (most recent call last): File "patchew-tester/src/patchew-cli", line 521, in test_one git_clone_repo(clone, r["repo"], r["head"], logf, True) File "patchew-tester/src/patchew-cli", line 48, in git_clone_repo stdout=logf, stderr=logf) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1. The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4...@amsat.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 28/36] tcg: Implement gvec support for rotate by immediate
Richard Henderson writes: > No host backend support yet, but the interfaces for rotli > are in place. Canonicalize immediate rotate to the left, > based on a survey of architectures, but provide both left > and right shift interfaces to the translators. > > Signed-off-by: Richard Henderson Reviewed-by: Alex Bennée -- Alex Bennée
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's ID_AA64DFR0
Patchew URL: https://patchew.org/QEMU/20200423110915.10527-1-peter.mayd...@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH] target/arm: Use correct variable for setting 'max' cpu's ID_AA64DFR0 Message-id: 20200423110915.10527-1-peter.mayd...@linaro.org Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 error: RPC failed; result=22, HTTP code = 504 fatal: The remote end hung up unexpectedly error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384 Traceback (most recent call last): File "patchew-tester/src/patchew-cli", line 521, in test_one git_clone_repo(clone, r["repo"], r["head"], logf, True) File "patchew-tester/src/patchew-cli", line 48, in git_clone_repo stdout=logf, stderr=logf) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1. The full log is available at http://patchew.org/logs/20200423110915.10527-1-peter.mayd...@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4...@amsat.org/ Hi, This series failed the docker-mingw@fedora build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #! /bin/bash export ARCH=x86_64 make docker-image-fedora V=1 NETWORK=1 time make docker-test-mingw@fedora J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4...@amsat.org/testing.docker-mingw@fedora/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4...@amsat.org/ Hi, This series failed the docker-quick@centos7 build test. Please find the testing commands and their output below. If you have Docker installed, you can probably reproduce it locally. === TEST SCRIPT BEGIN === #!/bin/bash make docker-image-centos7 V=1 NETWORK=1 time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4...@amsat.org/testing.docker-quick@centos7/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
Patchew URL: https://patchew.org/QEMU/20200423124305.14718-1-f4...@amsat.org/ Hi, This series failed build test on FreeBSD host. Please find the details below. === TEST SCRIPT BEGIN === #!/bin/bash # Testing script will be invoked under the git checkout with # HEAD pointing to a commit that has the patches applied on top of "base" # branch if qemu-system-x86_64 --help >/dev/null 2>&1; then QEMU=qemu-system-x86_64 elif /usr/libexec/qemu-kvm --help >/dev/null 2>&1; then QEMU=/usr/libexec/qemu-kvm else exit 1 fi make vm-build-freebsd J=21 QEMU=$QEMU exit 0 === TEST SCRIPT END === The full log is available at http://patchew.org/logs/20200423124305.14718-1-f4...@amsat.org/testing.FreeBSD/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [PATCH v2 29/36] tcg: Implement gvec support for rotate by vector
Richard Henderson writes: > No host backend support yet, but the interfaces for rotlv > and rotrv are in place. > > Signed-off-by: Richard Henderson Reviewed-by: Alex Bennée -- Alex Bennée
Re: [PATCH v2 31/36] tcg: Implement gvec support for rotate by scalar
Richard Henderson writes: > No host backend support yet, but the interfaces for rotls > are in place. Only implement left-rotate for now, as the > only known use of vector rotate by scalar is s390x, so any > right-rotate would be unused and untestable. > > Signed-off-by: Richard Henderson Reviewed-by: Alex Bennée -- Alex Bennée
Re: [PATCH v2 00/36] tcg 5.1 omnibus patch set
Richard Henderson writes: > For v1, I had split this into 4 logically distinct parts. But > apparently there are minor interdependencies, because the later > sets would not apply standalone, says Alex. > > Rather than tease them apart, and then have to undo that work > in order to actually apply them later, I'll just lump them. > > So: > > Part 1, patches 1-7, tcg_gen_gvec_dup_imm, is reviewed. > > Part 2, patch 8, vector tail clearing, is reviewed, and I have > moved the target/arm patches into a different queue. > > Part 3, patches 9-25, TYPE_CONST temporaries, is mostly unreviewed. > > Part 4, patch 26, load_dest for GVecGen2, a support patch for SVE2. > > Part 5, patches 27-36, add vector rotate patterns, is brand new. > I include two demonstrators for target/ppc and target/s390x. I've done my review pass for now. I made it through all the core code but my brain was too frazzled to look at the back end generation code so I'll have another go at that on the next revision when the sparc regression is figured out. -- Alex Bennée
Re: [PATCH RESEND v6 36/36] multi-process: add configure and usage information
Does multi-process support on Windows? I found it use mmap and unix socket for inter-process communication, that may not support under Windows. And also, can the python script be replaced by C implementation? On Thu, Apr 23, 2020 at 12:38 PM wrote: > From: Elena Ufimtseva > > Signed-off-by: Elena Ufimtseva > Signed-off-by: Jagannathan Raman > Signed-off-by: John G Johnson > --- > MAINTAINERS | 2 + > docs/multi-process.rst | 85 + > scripts/mpqemu-launcher-perf-mode.py | 92 > scripts/mpqemu-launcher.py | 53 > 4 files changed, 232 insertions(+) > create mode 100644 docs/multi-process.rst > create mode 100755 scripts/mpqemu-launcher-perf-mode.py > create mode 100755 scripts/mpqemu-launcher.py > > diff --git a/MAINTAINERS b/MAINTAINERS > index ed48615e15..8ff3bfae6a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2880,6 +2880,8 @@ F: remote/iohub.c > F: remote/remote-opts.h > F: remote/remote-opts.c > F: docs/devel/multi-process.rst > +F: scripts/mpqemu-launcher.py > +F: scripts/mpqemu-launcher-perf-mode.py > > Build and test automation > - > diff --git a/docs/multi-process.rst b/docs/multi-process.rst > new file mode 100644 > index 00..8387d6c691 > --- /dev/null > +++ b/docs/multi-process.rst > @@ -0,0 +1,85 @@ > +Multi-process QEMU > +== > + > +This document describes how to configure and use multi-process qemu. > +For the design document refer to docs/devel/qemu-multiprocess. > + > +1) Configuration > + > + > +To enable support for multi-process add --enable-mpqemu > +to the list of options for the "configure" script. > + > + > +2) Usage > + > + > +Multi-process QEMU requires an orchestrator to launch. Please refer to a > +light-weight python based orchestrator for mpqemu in > +scripts/mpqemu-launcher.py to lauch QEMU in multi-process mode. > + > +scripts/mpqemu-launcher-perf-mode.py launches in "perf" mode. In this > mode, > +the same QEMU process connects to multiple remote devices, each emulated > in > +a separate process. > + > +As of now, we only support the emulation of lsi53c895a in a separate > process. > + > +Following is a description of command-line used to launch mpqemu. > + > +* Orchestrator: > + > + - The Orchestrator creates a unix socketpair > + > + - It launches the remote process and passes one of the > +sockets to it via command-line. > + > + - It then launches QEMU and specifies the other socket as an option > +to the Proxy device object > + > +* Remote Process: > + > + - The first command-line option in the remote process is one of the > +sockets created by the Orchestrator > + > + - The remaining options are no different from how one launches QEMU with > +devices. The only other requirement is each PCI device must have a > +unique ID specified to it. This is needed to pair remote device with > the > +Proxy object. > + > + - Example command-line for the remote process is as follows: > + > + /usr/bin/qemu-scsu-dev 4 \ > + -device lsi53c895a,id=lsi0 \ > + -drive id=drive_image2,file=/build/ol7-nvme-test-1.qcow2 \ > + -device scsi-hd,id=drive2,drive=drive_image2,bus=lsi0.0,scsi-id=0 > + > +* QEMU: > + > + - Since parts of the RAM are shared between QEMU & remote process, a > +memory-backend-memfd is required to facilitate this, as follows: > + > +-object memory-backend-memfd,id=mem,size=2G > + > + - A "pci-proxy-dev" device is created for each of the PCI devices > emulated > +in the remote process. A "socket" sub-option specifies the other end > of > +unix channel created by orchestrator. The "id" sub-option must be > specified > +and should be the same as the "id" specified for the remote PCI device > + > + - Example commandline for QEMU is as follows: > + > + -device pci-proxy-dev,id=lsi0,socket=3 > + > +* Monitor / QMP: > + > + - The remote process supports QEMU monitor. It could be specified using > the > +"-monitor" or "-qmp" command-line options > + > + - As an example, one could connect to the monitor by adding the > following > +to the command-line of the remote process > + > + -monitor unix:/home/qmp-sock,server,nowait > + > + - The user could connect to the monitor using the qmp script or using > +"socat" as outlined below: > + > + socat /home/qmp-sock stdio > diff --git a/scripts/mpqemu-launcher-perf-mode.py > b/scripts/mpqemu-launcher-perf-mode.py > new file mode 100755 > index 00..2733424c76 > --- /dev/null > +++ b/scripts/mpqemu-launcher-perf-mode.py > @@ -0,0 +1,92 @@ > +#!/usr/bin/env python3 > + > +import socket > +import os > +import subprocess > +import time > + > +PROC_QEMU='/usr/bin/qemu-system-x86_64' > + > +PROC_REMOTE='/usr/bin/qemu-scsi-dev' > + > +proxy
Re: [PATCH] qcow2: Allow resize of images with internal snapshots
On 22.04.20 22:53, Eric Blake wrote:
> We originally refused to allow resize of images with internal
> snapshots because the v2 image format did not require the tracking of
> snapshot size, making it impossible to safely revert to a snapshot
> with a different size than the current view of the image. But the
> snapshot size tracking was rectified in v3, and our recent fixes to
> qemu-img amend (see 0a85af35) guarantee that we always have a valid
> snapshot size. Thus, we no longer need to artificially limit image
> resizes, but it does become one more thing that would prevent a
> downgrade back to v2. And now that we support different-sized
> snapshots, it's also easy to fix reverting to a snapshot to apply the
> new size.
>
> Upgrade iotest 61 to cover this (we previously had NO coverage of
> refusal to resize while snapshots exist). Note that the amend process
> can fail but still have effects: in particular, since we break things
> into upgrade, resize, downgrade, if a failure does not happen until a
> later phase (such as the downgrade attempt), earlier steps are still
> visible (a truncation and downgrade attempt will fail, but only after
> truncating data). But even before this patch, an attempt to upgrade
> and resize would fail but only after changing the image to v3. In
> some sense, partial image changes on failure are inevitible, since we
> can't avoid a mid-change EIO (if you are trying to amend more than one
> thing at once, but something fails, I hope you have a backup image).
>
> Signed-off-by: Eric Blake
> ---
> block/qcow2-snapshot.c | 21 --
> block/qcow2.c | 31 ++--
> tests/qemu-iotests/061 | 23 ++
> tests/qemu-iotests/061.out | 144 +
> 4 files changed, 211 insertions(+), 8 deletions(-)
>
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 82c32d4c9b08..3f9e48738d0b 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -23,6 +23,7 @@
> */
>
> #include "qemu/osdep.h"
> +#include "sysemu/block-backend.h"
> #include "qapi/error.h"
> #include "qcow2.h"
> #include "qemu/bswap.h"
> @@ -775,10 +776,22 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const
> char *snapshot_id)
> }
>
> if (sn->disk_size != bs->total_sectors * BDRV_SECTOR_SIZE) {
> -error_report("qcow2: Loading snapshots with different disk "
> -"size is not implemented");
> -ret = -ENOTSUP;
> -goto fail;
> +BlockBackend *blk = blk_new(bdrv_get_aio_context(bs),
> +BLK_PERM_RESIZE, BLK_PERM_ALL);
> +ret = blk_insert_bs(blk, bs, &local_err);
I wonder whether maybe we should reintroduce blk_new_with_bs().
> +if (ret < 0) {
> +blk_unref(blk);
> +error_report_err(local_err);
> +goto fail;
> +}
> +
> +ret = blk_truncate(blk, sn->disk_size, true, PREALLOC_MODE_OFF,
> + &local_err);
> +blk_unref(blk);
> +if (ret < 0) {
> +error_report_err(local_err);
> +goto fail;
> +}
> }
>
> /*
Looks good.
> diff --git a/block/qcow2.c b/block/qcow2.c
> index b524b0c53f84..29047c33b7e5 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3988,14 +3988,21 @@ static int coroutine_fn
> qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>
> qemu_co_mutex_lock(&s->lock);
>
> -/* cannot proceed if image has snapshots */
> -if (s->nb_snapshots) {
> -error_setg(errp, "Can't resize an image which has snapshots");
> +/*
> + * Even though we store snapshot size for all images, it was not
> + * required until v3, so it is not safe to proceed for v2.
> + */
> +if (s->nb_snapshots && s->qcow_version < 3) {
> +error_setg(errp, "Can't resize a v2 image which has snapshots");
> ret = -ENOTSUP;
> goto fail;
> }
>
> -/* cannot proceed if image has bitmaps */
> +/*
> + * For now, it's easier to not proceed if image has bitmaps, even
> + * though we could resize bitmaps, because it is not obvious
> + * whether new bits should be set or clear.
The previous comment was incorrect as well, but actually
qcow2_truncate_bitmaps_check() doesn’t return an error when there is any
bitmap, but only if there are non-active bitmaps, or active bitmaps that
cannot be modified. So for non-disabled bitmaps, we generally do
happily proceed.
> + */
> if (qcow2_truncate_bitmaps_check(bs, errp)) {
> ret = -ENOTSUP;
> goto fail;
[...]
> diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
> index ce285d308408..fdfb8fab5fb6 100755
> --- a/tests/qemu-iotests/061
> +++ b/tests/qemu-iotests/061
> @@ -111,6 +111,29 @@ $PYTHON qcow2.py "$TEST_IMG" dump-header
> $QEMU_IO -c "read -P 0x2a 42M 64k" "$TEST_IMG" | _filter_qemu_io
> _check_test_img
>
> +echo
> +echo "=== Testing resize with
Re: [PATCH v5 4/9] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate
On 23.04.20 15:25, Kevin Wolf wrote:
> Am 23.04.2020 um 12:53 hat Max Reitz geschrieben:
>> On 22.04.20 17:21, Kevin Wolf wrote:
>>> If BDRV_REQ_ZERO_WRITE is set and we're extending the image, calling
>>> qcow2_cluster_zeroize() with flags=0 does the right thing: It doesn't
>>> undo any previous preallocation, but just adds the zero flag to all
>>> relevant L2 entries. If an external data file is in use, a write_zeroes
>>> request to the data file is made instead.
>>>
>>> Signed-off-by: Kevin Wolf
>>> ---
>>> block/qcow2.c | 30 ++
>>> 1 file changed, 30 insertions(+)
>>>
>>> diff --git a/block/qcow2.c b/block/qcow2.c
>>> index 9cfbdfc939..bd632405d1 100644
>>> --- a/block/qcow2.c
>>> +++ b/block/qcow2.c
>>
>> [...]
>>
>>> @@ -4214,6 +4215,35 @@ static int coroutine_fn
>>> qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>>> g_assert_not_reached();
>>> }
>>>
>>> +if ((flags & BDRV_REQ_ZERO_WRITE) && offset > old_length) {
>>> +uint64_t zero_start = QEMU_ALIGN_UP(old_length, s->cluster_size);
>>> +uint64_t zero_end = QEMU_ALIGN_UP(offset, s->cluster_size);
>>> +
>>> +/* Use zero clusters as much as we can */
>>> +ret = qcow2_cluster_zeroize(bs, zero_start, zero_end - zero_start,
>>> 0);
>>
>> It’s kind of a pity that this changes the cluster mappings that were
>> established when using falloc/full preallocation already (i.e., they
>> become preallocated zero clusters then, so when writing to them, we need
>> COW again).
>>
>> But falloc/full preallocation do not guarantee that the new data is
>> zero, so I suppose this is the only thing we can reasonably do.
>
> If we really want, I guess we could make full preallocation first try
> passing BDRV_REQ_ZERO_WRITE to the protocol layer and if that succeeds,
> we could skip setting the zero cluster flag at the qcow2 level.
That might be nice.
> Feels like a separate patch, though.
Definitely, yes.
Max
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v5 4/9] qcow2: Support BDRV_REQ_ZERO_WRITE for truncate
On 4/23/20 8:23 AM, Kevin Wolf wrote: So qcow2_cluster_zeroize() seems to accept the unaligned tail. It would still set the zero flag for the partial last cluster and for the external data file, bdrv_co_pwrite_zeroes() would have the correct size. Then I'm in favor of NOT rounding the tail. That's an easy enough change and we've now justified that it does what we want, so R-b stands with that one-line tweak. Would have been too easy... bs->total_sectors isn't updated yet, so the assertion does fail. I can make the assertion check end_offset >= ... instead. That should still check what we wanted to check here and allow the unaligned extension. Yes, that works for me. This feels like the better option to me compared to updating bs->total_sectors earlier and then undoing that change in every error path. Indeed. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
