[Qemu-devel] Problems with MIPS Malta SSH tests in make check-acceptance

[David Gibson]

Hi,

I'm finding make check-acceptance is currently useless for me as a
pre-pull test, because a bunch of the tests are not at all reliable.
There are a bunch which I'm still investigating, but for now I'm
looking at the MIPS Malta SSH tests.

There seem to be at least two problems here.  First, the test includes
a download of a pretty big guest disk image.  This can easily exhaust
the 2m30 timeout on its own.

Even without the timeout, it makes the test really slow, even on
repeated runs.  Is there some way we can make the image download part
of "building" the tests rather than actually running the testsuite, so
that a) the test themselves go faster and b) we don't include the
download in the test timeout - obviously the download speed is hugely
dependent on factors that aren't really related to what we're testing
here.

In the meantime, I tried hacking it by just increasing the timeout to
10m.  That got several of the tests working for me, but one still
failed.  Specifically 'LinuxSSH.test_mips_malta32eb_kernel3_2_0' still
timed out for me, but now after booting the guest, rather than during
the image download.  Looking at the avocado log file I'm seeing a
bunch of soft lockup messages from the guest console, AFAICT.  So it
looks like we have a real bug here, which I suspect has been
overlooked precisely because the download problems mean this test
isn't reliable.

Any thoughts on how to improve the situation?

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[Qemu-devel] [RESEND PATCH] tests/acceptance: Specify arch for QueryCPUModelExpansion

[David Gibson]

At the moment this test runs on whatever the host arch is.  But it looks
for 'unavailable-features' which is an x86 specific cpu property.  Tag it
to always use qemu-system-x86_64.

Signed-off-by: David Gibson 
Reviewed-by: Wainer dos Santos Moschetta 
---
 tests/acceptance/cpu_queries.py | 3 +++
 1 file changed, 3 insertions(+)

I sent this a while back, but it seems to have been forgotten.  As far
as I can tell the current logic is Just Plain Wrong, on any host other
than x86.

diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
index e71edec39f..af47d2795a 100644
--- a/tests/acceptance/cpu_queries.py
+++ b/tests/acceptance/cpu_queries.py
@@ -18,6 +18,9 @@ class QueryCPUModelExpansion(Test):
 """
 
 def test(self):
+"""
+:avocado: tags=arch:x86_64
+"""
 self.vm.set_machine('none')
 self.vm.add_args('-S')
 self.vm.launch()
-- 
2.21.0

Re: [Qemu-devel] vhost, iova, and dirty page tracking

[Tian, Kevin]

> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Wednesday, September 18, 2019 2:04 PM
> 
> On 2019/9/18 上午9:31, Tian, Kevin wrote:
> >> From: Alex Williamson [mailto:alex.william...@redhat.com]
> >> Sent: Tuesday, September 17, 2019 10:54 PM
> >>
> >> On Tue, 17 Sep 2019 08:48:36 +
> >> "Tian, Kevin"  wrote:
> >>
>  From: Jason Wang [mailto:jasow...@redhat.com]
>  Sent: Monday, September 16, 2019 4:33 PM
> 
> 
>  On 2019/9/16 上午9:51, Tian, Kevin wrote:
> > Hi, Jason
> >
> > We had a discussion about dirty page tracking in VFIO, when
> vIOMMU
> > is enabled:
> >
> > https://lists.nongnu.org/archive/html/qemu-devel/2019-
>  09/msg02690.html
> > It's actually a similar model as vhost - Qemu cannot interpose the
> fast-
>  path
> > DMAs thus relies on the kernel part to track and report dirty page
>  information.
> > Currently Qemu tracks dirty pages in GFN level, thus demanding a
>  translation
> > from IOVA to GPA. Then the open in our discussion is where this
>  translation
> > should happen. Doing the translation in kernel implies a device iotlb
>  flavor,
> > which is what vhost implements today. It requires potentially large
>  tracking
> > structures in the host kernel, but leveraging the existing log_sync
> flow
> >> in
>  Qemu.
> > On the other hand, Qemu may perform log_sync for every removal
> of
>  IOVA
> > mapping and then do the translation itself, then avoiding the GPA
>  awareness
> > in the kernel side. It needs some change to current Qemu log-sync
> >> flow,
>  and
> > may bring more overhead if IOVA is frequently unmapped.
> >
> > So we'd like to hear about your opinions, especially about how you
> >> came
> > down to the current iotlb approach for vhost.
>  We don't consider too much in the point when introducing vhost. And
>  before IOTLB, vhost has already know GPA through its mem table
>  (GPA->HVA). So it's nature and easier to track dirty pages at GPA level
>  then it won't any changes in the existing ABI.
> >>> This is the same situation as VFIO.
> >> It is?  VFIO doesn't know GPAs, it only knows HVA, HPA, and IOVA.  In
> >> some cases IOVA is GPA, but not all.
> > Well, I thought vhost has a similar design, that the index of its mem table
> > is GPA when vIOMMU is off and then becomes IOVA when vIOMMU is on.
> > But I may be wrong here. Jason, can you help clarify? I saw two
> > interfaces which poke the mem table: VHOST_SET_MEM_TABLE (for GPA)
> > and VHOST_IOTLB_UPDATE (for IOVA). Are they used exclusively or
> together?
> >
> 
> Actually, vhost maintains two interval trees, mem table GPA->HVA, and
> device IOTLB IOVA->HVA. Device IOTLB is only used when vIOMMU is
> enabled, and in that case mem table is used only when vhost need to
> track dirty pages (do reverse lookup of memtable to get HVA->GPA). So in
> conclusion, for datapath, they are used exclusively, but they need
> cowork for logging dirty pages when device IOTLB is enabled.
> 

OK. Then it's different from current VFIO design, which maintains only
one tree which is indexed by either GPA or IOVA exclusively, upon 
whether vIOMMU is in use. 

Thanks
Kevin

[Qemu-devel] [PATCH RESEND v4 1/2] x86/cpu: Add support for UMONITOR/UMWAIT/TPAUSE

[Tao Xu]

UMONITOR, UMWAIT and TPAUSE are a set of user wait instructions.
This patch adds support for user wait instructions in KVM. Availability
of the user wait instructions is indicated by the presence of the CPUID
feature flag WAITPKG CPUID.0x07.0x0:ECX[5]. User wait instructions may
be executed at any privilege level, and use IA32_UMWAIT_CONTROL MSR to
set the maximum time.

The patch enable the umonitor, umwait and tpause features in KVM.
Because umwait and tpause can put a (psysical) CPU into a power saving
state, by default we dont't expose it to kvm and enable it only when
guest CPUID has it. And use QEMU command-line "-overcommit cpu-pm=on"
(enable_cpu_pm is enabled), a VM can use UMONITOR, UMWAIT and TPAUSE
instructions. If the instruction causes a delay, the amount of time
delayed is called here the physical delay. The physical delay is first
computed by determining the virtual delay (the time to delay relative to
the VM’s timestamp counter). Otherwise, UMONITOR, UMWAIT and TPAUSE cause
an invalid-opcode exception(#UD).

The release document ref below link:
https://software.intel.com/sites/default/files/\
managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf

Co-developed-by: Jingqi Liu 
Signed-off-by: Jingqi Liu 
Signed-off-by: Tao Xu 
---

No changes in v4.
---
 target/i386/cpu.c | 3 ++-
 target/i386/cpu.h | 1 +
 target/i386/kvm.c | 4 
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9e0bac31e8..15f888b13f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1062,7 +1062,7 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = 
{
 .type = CPUID_FEATURE_WORD,
 .feat_names = {
 NULL, "avx512vbmi", "umip", "pku",
-NULL /* ospke */, NULL, "avx512vbmi2", NULL,
+NULL /* ospke */, "waitpkg", "avx512vbmi2", NULL,
 "gfni", "vaes", "vpclmulqdq", "avx512vnni",
 "avx512bitalg", NULL, "avx512-vpopcntdq", NULL,
 "la57", NULL, NULL, NULL,
@@ -5227,6 +5227,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx,
&cpu->mwait.ecx, &cpu->mwait.edx);
 env->features[FEAT_1_ECX] |= CPUID_EXT_MONITOR;
+env->features[FEAT_7_0_ECX] |= CPUID_7_0_ECX_WAITPKG;
 }
 }
 
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5f6e3a029a..33a0b8b365 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -673,6 +673,7 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_7_0_ECX_UMIP (1U << 2)
 #define CPUID_7_0_ECX_PKU  (1U << 3)
 #define CPUID_7_0_ECX_OSPKE(1U << 4)
+#define CPUID_7_0_ECX_WAITPKG  (1U << 5) /* UMONITOR/UMWAIT/TPAUSE 
Instructions */
 #define CPUID_7_0_ECX_VBMI2(1U << 6) /* Additional VBMI Instrs */
 #define CPUID_7_0_ECX_GFNI (1U << 8)
 #define CPUID_7_0_ECX_VAES (1U << 9)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 92069099ab..dc618e4062 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -400,6 +400,10 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, 
uint32_t function,
 if (host_tsx_blacklisted()) {
 ret &= ~(CPUID_7_0_EBX_RTM | CPUID_7_0_EBX_HLE);
 }
+} else if (function == 7 && index == 0 && reg == R_ECX) {
+if (enable_cpu_pm) {
+ret |= CPUID_7_0_ECX_WAITPKG;
+}
 } else if (function == 7 && index == 0 && reg == R_EDX) {
 /*
  * Linux v4.17-v4.20 incorrectly return ARCH_CAPABILITIES on SVM hosts.
-- 
2.20.1

[Qemu-devel] [PATCH RESEND v4 0/2] x86: Enable user wait instructions

[Tao Xu]

UMONITOR, UMWAIT and TPAUSE are a set of user wait instructions.

UMONITOR arms address monitoring hardware using an address. A store
to an address within the specified address range triggers the
monitoring hardware to wake up the processor waiting in umwait.

UMWAIT instructs the processor to enter an implementation-dependent
optimized state while monitoring a range of addresses. The optimized
state may be either a light-weight power/performance optimized state
(c0.1 state) or an improved power/performance optimized state
(c0.2 state).

TPAUSE instructs the processor to enter an implementation-dependent
optimized state c0.1 or c0.2 state and wake up when time-stamp counter
reaches specified timeout.

Availability of the user wait instructions is indicated by the presence
of the CPUID feature flag WAITPKG CPUID.0x07.0x0:ECX[5].

The patches enable the umonitor, umwait and tpause features in KVM.
Because umwait and tpause can put a (psysical) CPU into a power saving
state, by default we dont't expose it in kvm and provide a capability to
enable it. Use kvm capability to enable UMONITOR, UMWAIT and TPAUSE when
QEMU use "-overcommit cpu-pm=on, a VM can use UMONITOR, UMWAIT and TPAUSE
instructions. If the instruction causes a delay, the amount of time
delayed is called here the physical delay. The physical delay is first
computed by determining the virtual delay (the time to delay relative to
the VM’s timestamp counter). Otherwise, UMONITOR, UMWAIT and TPAUSE cause
an invalid-opcode exception(#UD).

The dependency KVM patch link:
https://lkml.org/lkml/2019/7/16/58

The release document ref below link:
https://software.intel.com/sites/default/files/\
managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf

Changelog:
v4:
Set IA32_UMWAIT_CONTROL 32bits
v3:
Simplify the patches, expose user wait instructions when the guest
has CPUID (Paolo)
v2:
Separated from the series
https://www.mail-archive.com/qemu-devel@nongnu.org/msg549526.html
Use kvm capability to enable UMONITOR, UMWAIT and TPAUSE when
QEMU use "-overcommit cpu-pm=on"
v1:
Sent out with MOVDIRI/MOVDIR64B instructions patches

Tao Xu (2):
  x86/cpu: Add support for UMONITOR/UMWAIT/TPAUSE
  target/i386: Add support for save/load IA32_UMWAIT_CONTROL MSR

 target/i386/cpu.c |  3 ++-
 target/i386/cpu.h |  3 +++
 target/i386/kvm.c | 17 +
 target/i386/machine.c | 20 
 4 files changed, 42 insertions(+), 1 deletion(-)

-- 
2.20.1

[Qemu-devel] [PATCH RESEND v4 2/2] target/i386: Add support for save/load IA32_UMWAIT_CONTROL MSR

[Tao Xu]

UMWAIT and TPAUSE instructions use 32bits IA32_UMWAIT_CONTROL at MSR
index E1H to determines the maximum time in TSC-quanta that the processor
can reside in either C0.1 or C0.2.

This patch is to Add support for save/load IA32_UMWAIT_CONTROL MSR in
guest.

Co-developed-by: Jingqi Liu 
Signed-off-by: Jingqi Liu 
Signed-off-by: Tao Xu 
---

Changes in v4:
Set IA32_UMWAIT_CONTROL 32bits
---
 target/i386/cpu.h |  2 ++
 target/i386/kvm.c | 13 +
 target/i386/machine.c | 20 
 3 files changed, 35 insertions(+)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 33a0b8b365..bcd1cbbfc0 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -451,6 +451,7 @@ typedef enum X86Seg {
 
 #define MSR_IA32_BNDCFGS0x0d90
 #define MSR_IA32_XSS0x0da0
+#define MSR_IA32_UMWAIT_CONTROL 0xe1
 
 #define XSTATE_FP_BIT   0
 #define XSTATE_SSE_BIT  1
@@ -1393,6 +1394,7 @@ typedef struct CPUX86State {
 uint16_t fpregs_format_vmstate;
 
 uint64_t xss;
+uint32_t umwait;
 
 TPRAccess tpr_access_type;
 
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index dc618e4062..e548379096 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -95,6 +95,7 @@ static bool has_msr_hv_stimer;
 static bool has_msr_hv_frequencies;
 static bool has_msr_hv_reenlightenment;
 static bool has_msr_xss;
+static bool has_msr_umwait;
 static bool has_msr_spec_ctrl;
 static bool has_msr_virt_ssbd;
 static bool has_msr_smi_count;
@@ -1907,6 +1908,9 @@ static int kvm_get_supported_msrs(KVMState *s)
 case MSR_IA32_XSS:
 has_msr_xss = true;
 break;
+case MSR_IA32_UMWAIT_CONTROL:
+has_msr_umwait = true;
+break;
 case HV_X64_MSR_CRASH_CTL:
 has_msr_hv_crash = true;
 break;
@@ -2457,6 +2461,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
 if (has_msr_xss) {
 kvm_msr_entry_add(cpu, MSR_IA32_XSS, env->xss);
 }
+if (has_msr_umwait) {
+kvm_msr_entry_add(cpu, MSR_IA32_UMWAIT_CONTROL, env->umwait);
+}
 if (has_msr_spec_ctrl) {
 kvm_msr_entry_add(cpu, MSR_IA32_SPEC_CTRL, env->spec_ctrl);
 }
@@ -2861,6 +2868,9 @@ static int kvm_get_msrs(X86CPU *cpu)
 if (has_msr_xss) {
 kvm_msr_entry_add(cpu, MSR_IA32_XSS, 0);
 }
+if (has_msr_umwait) {
+kvm_msr_entry_add(cpu, MSR_IA32_UMWAIT_CONTROL, 0);
+}
 if (has_msr_spec_ctrl) {
 kvm_msr_entry_add(cpu, MSR_IA32_SPEC_CTRL, 0);
 }
@@ -3113,6 +3123,9 @@ static int kvm_get_msrs(X86CPU *cpu)
 case MSR_IA32_XSS:
 env->xss = msrs[i].data;
 break;
+case MSR_IA32_UMWAIT_CONTROL:
+env->umwait = msrs[i].data;
+break;
 default:
 if (msrs[i].index >= MSR_MC0_CTL &&
 msrs[i].index < MSR_MC0_CTL + (env->mcg_cap & 0xff) * 4) {
diff --git a/target/i386/machine.c b/target/i386/machine.c
index 2767b3096d..6481f846f6 100644
--- a/target/i386/machine.c
+++ b/target/i386/machine.c
@@ -943,6 +943,25 @@ static const VMStateDescription vmstate_xss = {
 }
 };
 
+static bool umwait_needed(void *opaque)
+{
+X86CPU *cpu = opaque;
+CPUX86State *env = &cpu->env;
+
+return env->umwait != 0;
+}
+
+static const VMStateDescription vmstate_umwait = {
+.name = "cpu/umwait",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = umwait_needed,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(env.umwait, X86CPU),
+VMSTATE_END_OF_LIST()
+}
+};
+
 #ifdef TARGET_X86_64
 static bool pkru_needed(void *opaque)
 {
@@ -1391,6 +1410,7 @@ VMStateDescription vmstate_x86_cpu = {
 &vmstate_msr_hyperv_reenlightenment,
 &vmstate_avx512,
 &vmstate_xss,
+&vmstate_umwait,
 &vmstate_tsc_khz,
 &vmstate_msr_smi_count,
 #ifdef TARGET_X86_64
-- 
2.20.1

Re: [Qemu-devel] [PATCH] * include/fpu/softfloat.h (floatx80_invalid_encoding): Handle m68k specific infinity pattern.

[Thomas Huth]

On 17/09/2019 22.04, Pierre Muller wrote:
> 
> Hello,
> 
>   I submitted September 12. a bug report about wrong handling of
> infinity values for m68k emulator.
> 
> https://bugs.launchpad.net/qemu/+bug/1843651
> 
>   The analysis I made in the bug report is wrong, because
> I did not know that m68k FPU really uses 80-bit representations internally.
>   It thus seems now to me that the m68k specific floatx80_infinity_low
> corresponds to a different internal encoding of infinity on m68k floating
> point units, different from the one of the usual x87 floating point unit.
>   The main problem still remains that the function floatx80_invalid_encoding
> does not properly handle those m68k infinity patterns.
> 
>   The patch below seems to fix the reported problem on current git.
> 
> Pierre Muller
> Member of core development team of Free Pascal compiler
> 
> 
> From e053d3d07b1951faf0b4637ce1ec4194b8d952e5 Mon Sep 17 00:00:00 2001
> From: Pierre Muller 
> Date: Thu, 12 Sep 2019 08:10:48 +
> Subject: [PATCH]* include/fpu/softfloat.h (floatx80_invalid_encoding):
>  Handle m68k specific infinity pattern.

 Hi Pierre,

thanks a lot for the patch! But please note that the QEMU project has
some constraints for patches that have to be followed before a patch can
be applied.

Most important one: You need to provide a "Signed-off-by:" line in the
patch description to make sure that you agree with the Developer
Certificate Of Origin. See this URL for more details:

 https://wiki.qemu.org/Contribute/SubmitAPatch

Then it would be nice if you add some proper commit message to the patch
(something similar to the comment that you've added to the source code
would do the job, I guess).

And finally, please note that qemu-devel is a high traffic mailing list.
When sending patches, you best make sure to put some maintainers on CC:,
or your patch might get lost in the high traffic. You can either have a
look at the MAINTAINERS file in the main directory, or use the
scripts/get_maintainers.pl script on your patch to see who should be put
on CC:.

 Thanks,
  Thomas


> ---
>  include/fpu/softfloat.h | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index ecb8ba0114..538c35767e 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -685,10 +685,17 @@ static inline int floatx80_is_any_nan(floatx80 a)
>  | pseudo-infinities and un-normal numbers. It does not include
>  | pseudo-denormals, which must still be correctly handled as inputs even
>  | if they are never generated as outputs.
> +| As m68k uses a different pattern for infinity, thus an additional test
> +| for valid infinity value must be added for m68k CPU.
>  
> **/
>  static inline bool floatx80_invalid_encoding(floatx80 a)
>  {
> +#if defined (TARGET_M68K)
> +return ((a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0)
> +   && (! floatx80_is_infinity(a));
> +#else
>  return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
> +#endif
>  }
> 
>  #define floatx80_zero make_floatx80(0x, 0xLL)
> --
> 2.20.1
> 
> 

Re: [Qemu-devel] [PATCH 02/17] block: Pass local error object pointer to error_append_hint()

[Kevin Wolf]

Am 17.09.2019 um 21:10 hat John Snow geschrieben:
> 
> 
> On 9/17/19 10:46 AM, Kevin Wolf wrote:
> > Am 17.09.2019 um 16:39 hat Eric Blake geschrieben:
> >> On 9/17/19 5:20 AM, Greg Kurz wrote:
> >>> Ensure that hints are added even if errp is &error_fatal or &error_abort.
> >>>
> >>> Signed-off-by: Greg Kurz 
> >>> ---
> >>>  block/backup.c   |7 +--
> >>>  block/dirty-bitmap.c |7 +--
> >>>  block/file-posix.c   |   20 +---
> >>>  block/gluster.c  |   23 +++
> >>>  block/qcow.c |   10 ++
> >>>  block/qcow2.c|7 +--
> >>>  block/vhdx-log.c |7 +--
> >>>  block/vpc.c  |7 +--
> >>>  8 files changed, 59 insertions(+), 29 deletions(-)
> >>>
> >>> diff --git a/block/backup.c b/block/backup.c
> >>> index 763f0d7ff6db..d8c422a0e3bc 100644
> >>> --- a/block/backup.c
> >>> +++ b/block/backup.c
> >>> @@ -602,11 +602,14 @@ static int64_t 
> >>> backup_calculate_cluster_size(BlockDriverState *target,
> >>>  BACKUP_CLUSTER_SIZE_DEFAULT);
> >>>  return BACKUP_CLUSTER_SIZE_DEFAULT;
> >>>  } else if (ret < 0 && !target->backing) {
> >>> -error_setg_errno(errp, -ret,
> >>> +Error *local_err = NULL;
> >>
> >> Can we go with the shorter name 'err' instead of 'local_err'?  I know,
> >> we aren't consistent (both styles appear throughout the tree), but the
> >> shorter style is more appealing to me.
> > 
> > I like local_err better because it's easier to distinguish from errp.
> > The compiler might catch it if you use the wrong one because one is
> > Error* and the other is Error**, but as a reviewer, I can still get
> > confused.
> 
> Doesn't that sound like a striking reason for condemnation of this
> entire model?

Might be, but do you have a better idea?

Kevin

[Qemu-devel] KVM call for 2019-09-24

[Juan Quintela]

Hi

Please, send any topic that you are interested in covering.

At the end of Monday I will send an email with the agenda or the
cancellation of the call, so hurry up.

After discussions on the QEMU Summit, we are going to have always open a
KVM call where you can add topics.

 Call details:

By popular demand, a google calendar public entry with it

  
https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ

(Let me know if you have any problems with the calendar entry.  I just
gave up about getting right at the same time CEST, CET, EDT and DST).

If you need phone number details,  contact me privately

Thanks, Juan.

Re: [Qemu-devel] [PATCH v3 18/29] s390x/tcg: MVCS/MVCP: Use access_memmove()

[David Hildenbrand]

On 17.09.19 22:20, Richard Henderson wrote:
> On 9/16/19 9:57 AM, David Hildenbrand wrote:
>> As we are moving between address spaces, we can use access_memmove_idx()
>> without checking for destructive overlaps (especially of real storage
>> locations):
>> "Each storage operand is processed left to right. The
>> storage-operand-consistency rules are the same as
>> for MOVE (MVC), except that when the operands
>> overlap in real storage, the use of the common real-
>> storage locations is not necessarily recognized."
>>
>> Signed-off-by: David Hildenbrand 
>> ---
>>  target/s390x/mem_helper.c | 26 --
>>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> Comment references access_memmove_idx.

Indeed, thanks!

> 
> Reviewed-by: Richard Henderson 
> 
> 
> r~
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] vhost, iova, and dirty page tracking

[Tian, Kevin]

> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Wednesday, September 18, 2019 2:10 PM
> 
> On 2019/9/18 上午9:44, Tian, Kevin wrote:
> >> From: Jason Wang [mailto:jasow...@redhat.com]
> >> Sent: Tuesday, September 17, 2019 6:36 PM
> >>
> >> On 2019/9/17 下午4:48, Tian, Kevin wrote:
>  From: Jason Wang [mailto:jasow...@redhat.com]
>  Sent: Monday, September 16, 2019 4:33 PM
> 
> 
>  On 2019/9/16 上午9:51, Tian, Kevin wrote:
> > Hi, Jason
> >
> > We had a discussion about dirty page tracking in VFIO, when
> vIOMMU
> > is enabled:
> >
> > https://lists.nongnu.org/archive/html/qemu-devel/2019-
>  09/msg02690.html
> > It's actually a similar model as vhost - Qemu cannot interpose the
> fast-
>  path
> > DMAs thus relies on the kernel part to track and report dirty page
>  information.
> > Currently Qemu tracks dirty pages in GFN level, thus demanding a
>  translation
> > from IOVA to GPA. Then the open in our discussion is where this
>  translation
> > should happen. Doing the translation in kernel implies a device iotlb
>  flavor,
> > which is what vhost implements today. It requires potentially large
>  tracking
> > structures in the host kernel, but leveraging the existing log_sync
> flow
> >> in
>  Qemu.
> > On the other hand, Qemu may perform log_sync for every removal
> of
>  IOVA
> > mapping and then do the translation itself, then avoiding the GPA
>  awareness
> > in the kernel side. It needs some change to current Qemu log-sync
> flow,
>  and
> > may bring more overhead if IOVA is frequently unmapped.
> >
> > So we'd like to hear about your opinions, especially about how you
> >> came
> > down to the current iotlb approach for vhost.
>  We don't consider too much in the point when introducing vhost. And
>  before IOTLB, vhost has already know GPA through its mem table
>  (GPA->HVA). So it's nature and easier to track dirty pages at GPA level
>  then it won't any changes in the existing ABI.
> >>> This is the same situation as VFIO.
> >>>
>  For VFIO case, the only advantages of using GPA is that the log can
> then
>  be shared among all the devices that belongs to the VM. Otherwise
>  syncing through IOVA is cleaner.
> >>> I still worry about the potential performance impact with this approach.
> >>> In current mdev live migration series, there are multiple system calls
> >>> involved when retrieving the dirty bitmap information for a given
> memory
> >>> range.
> >>
> >> I haven't took a deep look at that series. Technically dirty bitmap
> >> could be shared between device and driver, then there's no system call
> >> in synchronization.
> > That series require Qemu to tell the kernel about the information
> > about queried region (start, number, and page_size), read
> > the information about the dirty bitmap (offset, size) and then read
> > the dirty bitmap.
> 
> 
> Any pointer to that series, I can only find a "mdev live migration
> support with vfio-mdev-pci" from Liu Yi without actual codes.

https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg05543.html
It's interesting that I cannot google it. Have to manually find it in
Qemu archive.

> 
> 
> > Although the bitmap can be mmaped thus shared,
> > earlier reads/writes are conducted by pread/pwrite system calls.
> > This design is fine for current log_dirty implementation, where
> > dirty bitmap is synced in every pre-copy round. But to do it for
> > every IOVA unmap, it's definitely over-killed.
> >
> >>
> >>> IOVA mappings might be changed frequently. Though one may
> >>> argue that frequent IOVA change already has bad performance, it's still
> >>> not good to introduce further non-negligible overhead in such situation.
> >>
> >> Yes, it depends on the behavior of vIOMMU driver, e.g the frequency
> and
> >> granularity of the flushing.
> >>
> >>
> >>> On the other hand, I realized that adding IOVA awareness in VFIO is
> >>> actually easy. Today VFIO already maintains a full list of IOVA and its
> >>> associated HVA in vfio_dma structure, according to VFIO_MAP and
> >>> VFIO_UNMAP. As long as we allow the latter two operations to accept
> >>> another parameter (GPA), IOVA->GPA mapping can be naturally cached
> >>> in existing vfio_dma objects.
> >>
> >> Note that the HVA to GPA mapping is not an 1:1 mapping. One HVA
> range
> >> could be mapped to several GPA ranges.
> > This is fine. Currently vfio_dma maintains IOVA->HVA mapping.
> >
> > btw under what condition HVA->GPA is not 1:1 mapping? I didn't realize it.
> 
> 
> I don't remember the details e.g memory region alias? And neither kvm
> nor kvm API does forbid this if my memory is correct.
> 

I did see such comment in vhost code (log_write_hva):

/* More than one GPAs can be mapped into a single HVA. So
 * iterate all possible umems here to be safe.
 */

and looks it tries t

Re: [Qemu-devel] [PATCH 03/13] hw: Move MC146818 device from hw/timer/ to hw/rtc/ subdirectory

[Thomas Huth]

On 17/09/2019 12.03, Philippe Mathieu-Daudé wrote:
> On 9/17/19 7:07 AM, Thomas Huth wrote:
>> On 16/09/2019 17.48, Philippe Mathieu-Daudé wrote:
>>> The MC146818 is a Real Time Clock, not a timer.
>>> Move it under the hw/rtc/ subdirectory.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé 
>> [...]
>>> diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
>>> new file mode 100644
>>> index 00..888e04f9ab
>>> --- /dev/null
>>> +++ b/include/hw/rtc/mc146818rtc.h
>>> @@ -0,0 +1,38 @@
>>> +/*
>>> + * QEMU MC146818 RTC emulation
>>> + *
>>> + * Copyright (c) 2003-2004 Fabrice Bellard
>>> + *
>>> + * Permission is hereby granted, free of charge, to any person obtaining a 
>>> copy
>>> + * of this software and associated documentation files (the "Software"), 
>>> to deal
>>> + * in the Software without restriction, including without limitation the 
>>> rights
>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
>>> sell
>>> + * copies of the Software, and to permit persons to whom the Software is
>>> + * furnished to do so, subject to the following conditions:
>>> + *
>>> + * The above copyright notice and this permission notice shall be included 
>>> in
>>> + * all copies or substantial portions of the Software.
>>> + *
>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
>>> OR
>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
>>> OTHER
>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
>>> FROM,
>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
>>> IN
>>> + * THE SOFTWARE.
>>> + */
>>
>> If you run "git blame" on the old header file, it does not seem like
>> Fabrice wrote this header, so I'm not sure whether it makes sense to add
>> his (c) statement here?
>>
>> Maybe rather use a one-line "SPDX-License-Identifier: GPL-2.0-or-later"
>> here?
> 
> It was first added by Fabrice here:
> 
> $ git show 80cabfad163
[...]
> diff --git a/vl.h b/vl.h
> index 35962d1985..026a5dee5a 100644
> --- a/vl.h
> +++ b/vl.h
> +/* mc146818rtc.c */
> +
> +typedef struct RTCState {
> +uint8_t cmos_data[128];
> +uint8_t cmos_index;
> +int irq;
> +} RTCState;
> +
> +extern RTCState rtc_state;
> +
> +void rtc_init(int base, int irq);
> +void rtc_timer(void);

Ok, fair. But vl.h had a slightly different copyright statement than
vl.c, so I think you should rather use the one from vl.h.
But IMHO you could at least drop the "THE SOFTWARE IS PROVIDED ..."
paragraph and add a SPDX tag instead?

 Thomas

Re: [Qemu-devel] [PULL 0/8] Python queue, 2019-06-07

[Kevin Wolf]

Am 17.09.2019 um 23:48 hat Eduardo Habkost geschrieben:
> On Tue, Sep 17, 2019 at 03:57:26PM +0200, Kevin Wolf wrote:
> > Am 11.06.2019 um 19:12 hat Eduardo Habkost geschrieben:
> > > On Tue, Jun 11, 2019 at 05:07:55PM +0100, Peter Maydell wrote:
> > > > On Tue, 11 Jun 2019 at 17:03, Eduardo Habkost  
> > > > wrote:
> > > > >
> > > > > On Tue, Jun 11, 2019 at 04:50:34PM +0100, Peter Maydell wrote:
> > > > > > On Mon, 10 Jun 2019 at 13:58, Peter Maydell 
> > > > > >  wrote:
> > > > > > > Hi. This fails to build on one of my buildtest machines:
> > > > > > >
> > > > > > > ERROR: Cannot use 'python3', Python 2 >= 2.7 or Python 3 >= 3.5 
> > > > > > > is required.
> > > > > > >Use --python=/path/to/python to specify a supported Python.
> > > > > > >
> > > > > > > The machine has python 2.7.6 and 3.4.3. (It's an Ubuntu trusty
> > > > > > > box; it's one of the gcc compile farm machines so upgrades to its
> > > > > > > OS are not really under my control.)
> > > > > >
> > > > > > Rereading this, I realise that either the check or the error
> > > > > > message is wrong here. The machine has 2.7.6, which satisfies
> > > > > > "python 2 >= 2.7", so we should be OK to build. The bug
> > > > > > seems to be that we say "prefer python3 over plain python
> > > > > > on python2" early, but don't revisit that decision if the
> > > > > > python3 we found isn't actually good enough for us.
> > > > >
> > > > > Right.  The error message is technically correct, but misleading.
> > > > > python3 is too old, but python2 would work.
> > > > >
> > > > > We can make configure not use python3 by default if it's too old,
> > > > > and fall back to python2 in this case.
> > > > 
> > > > Sounds good. Since I have now managed to get my alternate
> > > > aarch64 box set up, how about I apply this pullreq and you
> > > > send a followup patch which does the fallback to python/python2 ?
> > > 
> > > I will remove the python2/python3 patches and send a new pull
> > > request.
> > 
> > What is the plan forward with this? Are the patches dropped for good?
> > 
> > I think the plan was to drop Python 2 after QEMU 4.2, and then it
> > becomes really relevant what our minimum Python 3 version is. We've just
> > had another Python version discussion in the context of iotests (John
> > suggested using function annotations, but these are >= 3.5 only).
> > 
> > Also, the fallback to Python 2 obviously makes no sense any more then,
> > so maybe it's not that important to add for a single QEMU release?
> > 
> > As Peter seems to have indicated above that he found a replacement for
> > the test machine with an OS that isn't out of support, can we just
> > revive this patch as it is?
> 
> My plan is to remove Python 2 support in QEMU 4.2 (making the
> fallback to Python 2 a non-issue), and require Python >= 3.5.

Then I think it would be best to make (or propose at least) that change
early in the release cycle. In other words, now. :-)

> Now, even if my plan is rejected and we keep supporting Python 2
> when building QEMU 4.2, my suggestion for the iotest maintainers
> is to make it require Python 3.5+ immediately, just like we do
> for tests/acceptance.  I don't see why we should keep wasting our
> energy supporting ancient Python versions in a test suite that is
> not a requirement for building QEMU.

Okay, if you as the Python maintainer say so, I'll gladly follow your
advice.

Maybe we can modify iotests so that it just skips Python tests if the
minimum version isn't available to keep the impact of deviating from the
global minimum version as small as possible. Of course, this will only
be necessary if your proposal to make 3.5 the minimum for all of QEMU is
rejected.

Kevin

Re: [Qemu-devel] [PATCH 02/17] block: Pass local error object pointer to error_append_hint()

[Greg Kurz]

On Tue, 17 Sep 2019 17:40:11 +
Vladimir Sementsov-Ogievskiy  wrote:

> 17.09.2019 18:37, Greg Kurz wrote:
> > On Tue, 17 Sep 2019 13:25:03 +
> > Vladimir Sementsov-Ogievskiy  wrote:
> > 
> >> 17.09.2019 13:20, Greg Kurz wrote:
> >>> Ensure that hints are added even if errp is &error_fatal or &error_abort.
> >>>
> >>> Signed-off-by: Greg Kurz 
> >>> ---
> >>>block/backup.c   |7 +--
> >>>block/dirty-bitmap.c |7 +--
> >>>block/file-posix.c   |   20 +---
> >>>block/gluster.c  |   23 +++
> >>>block/qcow.c |   10 ++
> >>>block/qcow2.c|7 +--
> >>>block/vhdx-log.c |7 +--
> >>>block/vpc.c  |7 +--
> >>>8 files changed, 59 insertions(+), 29 deletions(-)
> >>>
> >>> diff --git a/block/backup.c b/block/backup.c
> >>> index 763f0d7ff6db..d8c422a0e3bc 100644
> >>> --- a/block/backup.c
> >>> +++ b/block/backup.c
> >>> @@ -602,11 +602,14 @@ static int64_t 
> >>> backup_calculate_cluster_size(BlockDriverState *target,
> >>>BACKUP_CLUSTER_SIZE_DEFAULT);
> >>>return BACKUP_CLUSTER_SIZE_DEFAULT;
> >>>} else if (ret < 0 && !target->backing) {
> >>> -error_setg_errno(errp, -ret,
> >>> +Error *local_err = NULL;
> >>> +
> >>> +error_setg_errno(&local_err, -ret,
> >>>"Couldn't determine the cluster size of the target image, "
> >>>"which has no backing file");
> >>> -error_append_hint(errp,
> >>> +error_append_hint(&local_err,
> >>>"Aborting, since this may create an unusable destination 
> >>> image\n");
> >>> +error_propagate(errp, local_err);
> >>>return ret;
> >>>} else if (ret < 0 && target->backing) {
> >>>/* Not fatal; just trudge on ahead. */
> >>
> >>
> >> Pain.. Do we need these hints, if they are so painful?
> >>
> > 
> > I agree that the one above doesn't qualify as a useful hint.
> > It just tells that QEMU is giving up and gives no indication
> > to the user on how to avoid the issue. It should probably be
> > dropped.
> > 
> >> At least for cases like this, we can create helper function
> >>
> >> error_setg_errno_hint(..., error, hint)
> > 
> > Not very useful if hint has to be forged separately with
> > g_sprintf(), and we can't have such a thing as:
> > 
> > error_setg_errno_hint(errp, err_fmt, ..., hint_fmt, ...)
> > 
> >>
> >> But what could be done when we call function, which may or may not set 
> >> errp?
> >>
> >> ret = f(errp);
> >> if (ret) {
> >>  error_append_hint(errp, hint);
> >> }
> >>
> > 
> > Same problem. If errp is &error_fatal and f() does errno_setg(errp), it
> > ends up calling exit().
> > 
> >> Hmmm..
> >>
> >> Can it look like
> >>
> >> ret = f(..., hint_helper(errp, hint))
> >>
> >> ?
> >>
> > 
> > Nope, hint_helper() will get called before f() and things are worse.
> > If errp is NULL then error_append_hint() does nothing and if it is
> > &error_fatal then it aborts.
> > 
> >> I can't imagine how to do it, as someone should remove hint from 
> >> error_abort object on
> >> success path..
> >>
> >> But seems, the following is possible, which seems better for me than 
> >> local-error approach:
> >>
> >> error_push_hint(errp, hint);
> >> ret = f(.., errp);
> >> error_pop_hint(errp);
> >>
> > 
> > Matter of taste... also, it looks awkward to come up with a hint
> > before knowing what happened. I mean the appropriate hint could
> > depend on the value returned by f() and/or errno for example.
> > 
> >> ===
> >>
> >> Continue thinking on this:
> >>
> >> It may look like just
> >> ret = f(..., set_hint(errp, hint));
> >>
> >> or (just to split long line):
> >> set_hint(errp, hint);
> >> ret = f(..., errp);
> >>
> >> if in each function with errp does error_push_hint(errp) on start and 
> >> error_pop_hint(errp) on exit,
> >> which may be just one call at function start of macro, which will call 
> >> error_push_hint(errp) and
> >> define local variable by g_auto, with cleanup which will call 
> >> error_pop_hint(errp) on function
> >> exit..
> >>
> >> Or, may be, more direct way to set cleanup for function exists?
> >>
> >> ===
> >>
> >> Also, we can implement some code generation, to generate for functions 
> >> with errp argument
> >> wrappers with additional hint parameter, and just use these wrappers..
> >>
> >> ===
> >>
> >> If nobody likes any of my suggestions, then ignore them. I understand that 
> >> this series fixes
> >> real issue and much effort has already been spent. May be one day I'll try 
> >> to rewrite it...
> >>
> > 
> > For the reason exposed above, I don't think it makes sense to
> > build the hint before calling a function that calls error_setg().
> > I'm afraid we're stuck with local_err... it is then up to the
> > people to make it as less painful as possible.
> > 
> 
> Hmm. so, seems that in general we need local_err..
> 
> Then may be,

Re: [Qemu-devel] [PATCH v1 0/9] testing/next (docker, podman, float)

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190917184109.12564-1-alex.ben...@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH  v1 0/9] testing/next (docker,podman,float)
Message-id: 20190917184109.12564-1-alex.ben...@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 ===

From https://github.com/patchew-project/qemu
 * [new tag] patchew/20190917184109.12564-1-alex.ben...@linaro.org -> 
patchew/20190917184109.12564-1-alex.ben...@linaro.org
Switched to a new branch 'test'
484a149 tests/tcg: add refs for PPC float_[convs|madds] tests (FAILS TESTS)
5f79356 tests/tcg: add generic version of float_convs
c4eebe8 tests/tcg: add float_madds test to multiarch
a21855f tests/tcg: re-enable linux-test for ppc64abi32
ec04d62 tests/tcg: clean-up some comments after the de-tangling
6040174 target/ppc: fix signal delivery for ppc64abi32
e554591 podman: fix command invocation
de8a1e1 tests/docker: fix DOCKER_PARTIAL_IMAGES
4b02888 tests/docker: add sanitizers back to clang build

=== OUTPUT BEGIN ===
1/9 Checking commit 4b028882bb30 (tests/docker: add sanitizers back to clang 
build)
2/9 Checking commit de8a1e184fc1 (tests/docker: fix DOCKER_PARTIAL_IMAGES)
3/9 Checking commit e554591a7f1e (podman: fix command invocation)
4/9 Checking commit 60401740b757 (target/ppc: fix signal delivery for 
ppc64abi32)
5/9 Checking commit ec04d62eca41 (tests/tcg: clean-up some comments after the 
de-tangling)
6/9 Checking commit a21855f21eae (tests/tcg: re-enable linux-test for 
ppc64abi32)
7/9 Checking commit c4eebe811e75 (tests/tcg: add float_madds test to multiarch)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41: 
new file mode 100644

ERROR: do not initialise statics to 0 or NULL
#1942: FILE: tests/tcg/multiarch/float_helpers.c:59:
+static int num_extra_f16 = 0;

ERROR: do not initialise statics to 0 or NULL
#1943: FILE: tests/tcg/multiarch/float_helpers.c:60:
+static int alloc_f16 = 0;

ERROR: open brace '{' following function declarations go on the next line
#1946: FILE: tests/tcg/multiarch/float_helpers.c:63:
+int get_num_f16(void) {

ERROR: open brace '{' following function declarations go on the next line
#1950: FILE: tests/tcg/multiarch/float_helpers.c:67:
+void add_f16_const(uint16_t new) {

ERROR: open brace '{' following function declarations go on the next line
#1958: FILE: tests/tcg/multiarch/float_helpers.c:75:
+uint16_t get_f16(int i) {

ERROR: open brace '{' following function declarations go on the next line
#1968: FILE: tests/tcg/multiarch/float_helpers.c:85:
+char *fmt_16(uint16_t num) {

WARNING: architecture specific defines should be avoided
#1980: FILE: tests/tcg/multiarch/float_helpers.c:97:
+# if __GNUC_PREREQ(3, 3)

ERROR: space prohibited between function name and open parenthesis '('
#1981: FILE: tests/tcg/multiarch/float_helpers.c:98:
+#  define SNANF (__builtin_nansf (""))

ERROR: space prohibited between function name and open parenthesis '('
#1982: FILE: tests/tcg/multiarch/float_helpers.c:99:
+#  define SNAN (__builtin_nans (""))

ERROR: space prohibited between function name and open parenthesis '('
#1983: FILE: tests/tcg/multiarch/float_helpers.c:100:
+#  define SNANL (__builtin_nansl (""))

ERROR: spaces required around that '+' (ctx:VxV)
#1992: FILE: tests/tcg/multiarch/float_helpers.c:109:
+-0x1.1874b2p+103,
 ^

ERROR: spaces required around that '+' (ctx:VxV)
#1993: FILE: tests/tcg/multiarch/float_helpers.c:110:
+-0x1.c0bab6p+99,
 ^

ERROR: spaces required around that '-' (ctx:VxV)
#1994: FILE: tests/tcg/multiarch/float_helpers.c:111:
+-0x1.31f75p-40,
^

ERROR: spaces required around that '-' (ctx:VxV)
#1995: FILE: tests/tcg/multiarch/float_helpers.c:112:
+-0x1.505444p-66,
 ^

ERROR: spaces required around that '-' (ctx:VxV)
#1999: FILE: tests/tcg/multiarch/float_helpers.c:116:
+0x1p-25,
 ^

ERROR: spaces required around that '-' (ctx:VxV)
#2000: FILE: tests/tcg/multiarch/float_helpers.c:117:
+0x1.e6p-25, /* min positive FP16 subnormal */
^

ERROR: spaces required around that '-' (ctx:VxV)
#2001: FILE: tests/tcg/multiarch/float_helpers.c:118:
+0x1.ff801ap-15, /* max subnormal FP16 */
^

ERROR: spaces required around that '-' (ctx:VxV)
#2002: FILE: tests/tcg/multiarch/float_helpers.c:119:
+0x1.0cp-14, /* min positive normal FP16 */
^

ERROR: spaces required around that '+' (ctx:VxV)
#2004: FILE: tests/tcg/multiarch/float_helpers.c:121:
+0x1.004p+0, /* smallest float after 1.0 FP16 */
 ^

ERROR: spaces required around that '+' (ctx:VxV)
#2007: FILE: tests/tcg/multiarch/fl

[Qemu-devel] [PATCH v14 3/7] target/ppc: Handle NMI guest exit

[Aravinda Prasad]

Memory error such as bit flips that cannot be corrected
by hardware are passed on to the kernel for handling.
If the memory address in error belongs to guest then
the guest kernel is responsible for taking suitable action.
Patch [1] enhances KVM to exit guest with exit reason
set to KVM_EXIT_NMI in such cases. This patch handles
KVM_EXIT_NMI exit.

[1] https://www.spinics.net/lists/kvm-ppc/msg12637.html
(e20bbd3d and related commits)

Signed-off-by: Aravinda Prasad 
Reviewed-by: David Gibson 
Reviewed-by: Greg Kurz 
---
 hw/ppc/spapr.c  |8 
 hw/ppc/spapr_events.c   |   37 +
 include/hw/ppc/spapr.h  |   10 ++
 target/ppc/kvm.c|   14 ++
 target/ppc/kvm_ppc.h|2 ++
 target/ppc/trace-events |1 +
 6 files changed, 72 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 8288e8b..76ed988 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1823,6 +1823,12 @@ static void spapr_machine_reset(MachineState *machine)
 first_ppc_cpu->env.gpr[5] = 0;
 
 spapr->cas_reboot = false;
+
+spapr->mc_status = -1;
+spapr->guest_machine_check_addr = -1;
+
+/* Signal all vCPUs waiting on this condition */
+qemu_cond_broadcast(&spapr->mc_delivery_cond);
 }
 
 static void spapr_create_nvram(SpaprMachineState *spapr)
@@ -3099,6 +3105,8 @@ static void spapr_machine_init(MachineState *machine)
 
 kvmppc_spapr_enable_inkernel_multitce();
 }
+
+qemu_cond_init(&spapr->mc_delivery_cond);
 }
 
 static int spapr_kvm_type(MachineState *machine, const char *vm_type)
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 0e4c195..0ce96b8 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -40,6 +40,7 @@
 #include "hw/ppc/spapr_drc.h"
 #include "qemu/help_option.h"
 #include "qemu/bcd.h"
+#include "qemu/main-loop.h"
 #include "hw/ppc/spapr_ovec.h"
 #include 
 
@@ -621,6 +622,42 @@ void 
spapr_hotplug_req_remove_by_count_indexed(SpaprDrcType drc_type,
 RTAS_LOG_V6_HP_ACTION_REMOVE, drc_type, &drc_id);
 }
 
+void spapr_mce_req_event(PowerPCCPU *cpu)
+{
+SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+CPUState *cs = CPU(cpu);
+
+if (spapr->guest_machine_check_addr == -1) {
+/*
+ * This implies that we have hit a machine check either when the
+ * guest has not registered FWNMI (i.e., "ibm,nmi-register" not
+ * called) or between system reset and "ibm,nmi-register".
+ * Fall back to the old machine check behavior in such cases.
+ */
+cs->exception_index = POWERPC_EXCP_MCHECK;
+ppc_cpu_do_interrupt(cs);
+return;
+}
+
+while (spapr->mc_status != -1) {
+/*
+ * Check whether the same CPU got machine check error
+ * while still handling the mc error (i.e., before
+ * that CPU called "ibm,nmi-interlock")
+ */
+if (spapr->mc_status == cpu->vcpu_id) {
+qemu_system_guest_panicked(NULL);
+return;
+}
+qemu_cond_wait_iothread(&spapr->mc_delivery_cond);
+/* Meanwhile if the system is reset, then just return */
+if (spapr->guest_machine_check_addr == -1) {
+return;
+}
+}
+spapr->mc_status = cpu->vcpu_id;
+}
+
 static void check_exception(PowerPCCPU *cpu, SpaprMachineState *spapr,
 uint32_t token, uint32_t nargs,
 target_ulong args,
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 66049ac..99a2966 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -192,6 +192,15 @@ struct SpaprMachineState {
  * occurs during the unplug process. */
 QTAILQ_HEAD(, SpaprDimmState) pending_dimm_unplugs;
 
+/* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
+target_ulong guest_machine_check_addr;
+/*
+ * mc_status is set to -1 if mc is not in progress, else is set to the CPU
+ * handling the mc.
+ */
+int mc_status;
+QemuCond mc_delivery_cond;
+
 /*< public >*/
 char *kvm_type;
 char *host_model;
@@ -805,6 +814,7 @@ void spapr_clear_pending_events(SpaprMachineState *spapr);
 int spapr_max_server_number(SpaprMachineState *spapr);
 void spapr_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
   uint64_t pte0, uint64_t pte1);
+void spapr_mce_req_event(PowerPCCPU *cpu);
 
 /* DRC callbacks. */
 void spapr_core_release(DeviceState *dev);
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 8b1ab78..1e2a4f1 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1704,6 +1704,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run 
*run)
 ret = 0;
 break;
 
+case KVM_EXIT_NMI:
+trace_kvm_handle_nmi_exception();
+ret = kvm_handle_nmi(cpu, run);
+break;
+
 default:
 fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_re

[Qemu-devel] [PATCH v14 2/7] ppc: spapr: Introduce FWNMI capability

[Aravinda Prasad]

Introduce the KVM capability KVM_CAP_PPC_FWNMI so that
the KVM causes guest exit with NMI as exit reason
when it encounters a machine check exception on the
address belonging to a guest. Without this capability
enabled, KVM redirects machine check exceptions to
guest's 0x200 vector.

This patch also introduces fwnmi-mce capability to
deal with the case when a guest with the
KVM_CAP_PPC_FWNMI capability enabled is attempted
to migrate to a host that does not support this
capability.

Signed-off-by: Aravinda Prasad 
---
 hw/ppc/spapr.c |1 +
 hw/ppc/spapr_caps.c|   29 +
 include/hw/ppc/spapr.h |4 +++-
 target/ppc/kvm.c   |   26 ++
 target/ppc/kvm_ppc.h   |   12 
 5 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ea56499..8288e8b 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4487,6 +4487,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
 smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
 smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF;
+smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF;
 spapr_caps_add_properties(smc, &error_abort);
 smc->irq = &spapr_irq_dual;
 smc->dr_phb_enabled = true;
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 481dfd2..c11ff87 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -496,6 +496,25 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, 
uint8_t val,
 }
 }
 
+static void cap_fwnmi_mce_apply(SpaprMachineState *spapr, uint8_t val,
+Error **errp)
+{
+if (!val) {
+return; /* Disabled by default */
+}
+
+if (tcg_enabled()) {
+/*
+ * TCG support may not be correct in some conditions (e.g., in case
+ * of software injected faults like duplicate SLBs).
+ */
+warn_report("Firmware Assisted Non-Maskable Interrupts not supported 
in TCG");
+} else if (kvm_enabled() && !kvmppc_has_cap_ppc_fwnmi()) {
+error_setg(errp,
+"Firmware Assisted Non-Maskable Interrupts not supported by KVM, try 
cap-fwnmi-mce=off");
+}
+}
+
 SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
 [SPAPR_CAP_HTM] = {
 .name = "htm",
@@ -595,6 +614,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
 .type = "bool",
 .apply = cap_ccf_assist_apply,
 },
+[SPAPR_CAP_FWNMI_MCE] = {
+.name = "fwnmi-mce",
+.description = "Handle fwnmi machine check exceptions",
+.index = SPAPR_CAP_FWNMI_MCE,
+.get = spapr_cap_get_bool,
+.set = spapr_cap_set_bool,
+.type = "bool",
+.apply = cap_fwnmi_mce_apply,
+},
 };
 
 static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
@@ -734,6 +762,7 @@ SPAPR_CAP_MIG_STATE(hpt_maxpagesize, 
SPAPR_CAP_HPT_MAXPAGESIZE);
 SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV);
 SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER);
 SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST);
+SPAPR_CAP_MIG_STATE(fwnmi, SPAPR_CAP_FWNMI_MCE);
 
 void spapr_caps_init(SpaprMachineState *spapr)
 {
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 03111fd..66049ac 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -79,8 +79,10 @@ typedef enum {
 #define SPAPR_CAP_LARGE_DECREMENTER 0x08
 /* Count Cache Flush Assist HW Instruction */
 #define SPAPR_CAP_CCF_ASSIST0x09
+/* FWNMI machine check handling */
+#define SPAPR_CAP_FWNMI_MCE 0x0A
 /* Num Caps */
-#define SPAPR_CAP_NUM   (SPAPR_CAP_CCF_ASSIST + 1)
+#define SPAPR_CAP_NUM   (SPAPR_CAP_FWNMI_MCE + 1)
 
 /*
  * Capability Values
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 8c5b1f2..8b1ab78 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -85,6 +85,7 @@ static int cap_ppc_safe_indirect_branch;
 static int cap_ppc_count_cache_flush_assist;
 static int cap_ppc_nested_kvm_hv;
 static int cap_large_decr;
+static int cap_ppc_fwnmi;
 
 static uint32_t debug_inst_opcode;
 
@@ -2055,6 +2056,26 @@ void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int 
mpic_proxy)
 }
 }
 
+int kvmppc_set_fwnmi(void)
+{
+PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+CPUState *cs = CPU(cpu);
+int ret;
+
+ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
+if (ret) {
+error_report("This KVM version does not support FWNMI");
+return ret;
+}
+
+/*
+ * cap_ppc_fwnmi is set when FWNMI is available and enabled in KVM
+ * and not just when FWNMI is available in KVM
+ */
+cap_ppc_fwnmi = 1;
+return ret;
+}
+
 int kvmppc_smt_threads(void)
 {
 return cap_ppc_smt ? cap_ppc_smt : 1;
@@ -2355,6 +2376,11 @@ bool kvmppc_has_cap_mmu_hash_v3(void)
 return ca

[Qemu-devel] [PATCH v14 4/7] target/ppc: Build rtas error log upon an MCE

[Aravinda Prasad]

Upon a machine check exception (MCE) in a guest address space,
KVM causes a guest exit to enable QEMU to build and pass the
error to the guest in the PAPR defined rtas error log format.

This patch builds the rtas error log, copies it to the rtas_addr
and then invokes the guest registered machine check handler. The
handler in the guest takes suitable action(s) depending on the type
and criticality of the error. For example, if an error is
unrecoverable memory corruption in an application inside the
guest, then the guest kernel sends a SIGBUS to the application.
For recoverable errors, the guest performs recovery actions and
logs the error.

Signed-off-by: Aravinda Prasad 
---
 hw/ppc/spapr.c |   13 +++
 hw/ppc/spapr_events.c  |  222 
 hw/ppc/spapr_rtas.c|   26 ++
 include/hw/ppc/spapr.h |6 +
 target/ppc/kvm.c   |4 +
 5 files changed, 268 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 76ed988..9f2e5d2 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2930,6 +2930,19 @@ static void spapr_machine_init(MachineState *machine)
 error_report("Could not get size of LPAR rtas '%s'", filename);
 exit(1);
 }
+
+if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) {
+/*
+ * Ensure that the rtas image size is less than RTAS_ERROR_LOG_OFFSET
+ * or else the rtas image will be overwritten with the rtas error log
+ * when a machine check exception is encountered.
+ */
+g_assert(spapr->rtas_size < RTAS_ERROR_LOG_OFFSET);
+
+/* Resize rtas blob to accommodate error log */
+spapr->rtas_size = RTAS_ERROR_LOG_MAX;
+}
+
 spapr->rtas_blob = g_malloc(spapr->rtas_size);
 if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
 error_report("Could not load LPAR rtas '%s'", filename);
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 0ce96b8..ecc3d68 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -214,6 +214,106 @@ struct hp_extended_log {
 struct rtas_event_log_v6_hp hp;
 } QEMU_PACKED;
 
+struct rtas_event_log_v6_mc {
+#define RTAS_LOG_V6_SECTION_ID_MC   0x4D43 /* MC */
+struct rtas_event_log_v6_section_header hdr;
+uint32_t fru_id;
+uint32_t proc_id;
+uint8_t error_type;
+#define RTAS_LOG_V6_MC_TYPE_UE   0
+#define RTAS_LOG_V6_MC_TYPE_SLB  1
+#define RTAS_LOG_V6_MC_TYPE_ERAT 2
+#define RTAS_LOG_V6_MC_TYPE_TLB  4
+#define RTAS_LOG_V6_MC_TYPE_D_CACHE  5
+#define RTAS_LOG_V6_MC_TYPE_I_CACHE  7
+uint8_t sub_err_type;
+#define RTAS_LOG_V6_MC_UE_INDETERMINATE  0
+#define RTAS_LOG_V6_MC_UE_IFETCH 1
+#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_IFETCH 2
+#define RTAS_LOG_V6_MC_UE_LOAD_STORE 3
+#define RTAS_LOG_V6_MC_UE_PAGE_TABLE_WALK_LOAD_STORE 4
+#define RTAS_LOG_V6_MC_SLB_PARITY0
+#define RTAS_LOG_V6_MC_SLB_MULTIHIT  1
+#define RTAS_LOG_V6_MC_SLB_INDETERMINATE 2
+#define RTAS_LOG_V6_MC_ERAT_PARITY   1
+#define RTAS_LOG_V6_MC_ERAT_MULTIHIT 2
+#define RTAS_LOG_V6_MC_ERAT_INDETERMINATE3
+#define RTAS_LOG_V6_MC_TLB_PARITY1
+#define RTAS_LOG_V6_MC_TLB_MULTIHIT  2
+#define RTAS_LOG_V6_MC_TLB_INDETERMINATE 3
+uint8_t reserved_1[6];
+uint64_t effective_address;
+uint64_t logical_address;
+} QEMU_PACKED;
+
+struct mc_extended_log {
+struct rtas_event_log_v6 v6hdr;
+struct rtas_event_log_v6_mc mc;
+} QEMU_PACKED;
+
+struct MC_ierror_table {
+unsigned long srr1_mask;
+unsigned long srr1_value;
+bool nip_valid; /* nip is a valid indicator of faulting address */
+uint8_t error_type;
+uint8_t error_subtype;
+unsigned int initiator;
+unsigned int severity;
+};
+
+static const struct MC_ierror_table mc_ierror_table[] = {
+{ 0x081c, 0x0004, true,
+  RTAS_LOG_V6_MC_TYPE_UE, RTAS_LOG_V6_MC_UE_IFETCH,
+  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x081c, 0x0008, true,
+  RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_PARITY,
+  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x081c, 0x000c, true,
+  RTAS_LOG_V6_MC_TYPE_SLB, RTAS_LOG_V6_MC_SLB_MULTIHIT,
+  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x081c, 0x0010, true,
+  RTAS_LOG_V6_MC_TYPE_ERAT, RTAS_LOG_V6_MC_ERAT_MULTIHIT,
+  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVERITY_ERROR_SYNC, },
+{ 0x081c, 0x0014, true,
+  RTAS_LOG_V6_MC_TYPE_TLB, RTAS_LOG_V6_MC_TLB_MULTIHIT,
+  RTAS_LOG_INITIATOR_CPU, RTAS_LOG_SEVER

[Qemu-devel] [PATCH v14 1/7] Wrapper function to wait on condition for the main loop mutex

[Aravinda Prasad]

Introduce a wrapper function to wait on condition for
the main loop mutex. This function atomically releases
the main loop mutex and causes the calling thread to
block on the condition. This wrapper is required because
qemu_global_mutex is a static variable.

Signed-off-by: Aravinda Prasad 
Reviewed-by: David Gibson 
Reviewed-by: Greg Kurz 
---
 cpus.c   |5 +
 include/qemu/main-loop.h |8 
 2 files changed, 13 insertions(+)

diff --git a/cpus.c b/cpus.c
index 85cd451..d229830 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1877,6 +1877,11 @@ void qemu_mutex_unlock_iothread(void)
 qemu_mutex_unlock(&qemu_global_mutex);
 }
 
+void qemu_cond_wait_iothread(QemuCond *cond)
+{
+qemu_cond_wait(cond, &qemu_global_mutex);
+}
+
 static bool all_vcpus_paused(void)
 {
 CPUState *cpu;
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index f6ba78e..a6d20b0 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -295,6 +295,14 @@ void qemu_mutex_lock_iothread_impl(const char *file, int 
line);
  */
 void qemu_mutex_unlock_iothread(void);
 
+/*
+ * qemu_cond_wait_iothread: Wait on condition for the main loop mutex
+ *
+ * This function atomically releases the main loop mutex and causes
+ * the calling thread to block on the condition.
+ */
+void qemu_cond_wait_iothread(QemuCond *cond);
+
 /* internal interfaces */
 
 void qemu_fd_register(int fd);

[Qemu-devel] [PATCH v14 5/7] ppc: spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls

[Aravinda Prasad]

This patch adds support in QEMU to handle "ibm,nmi-register"
and "ibm,nmi-interlock" RTAS calls.

The machine check notification address is saved when the
OS issues "ibm,nmi-register" RTAS call.

This patch also handles the case when multiple processors
experience machine check at or about the same time by
handling "ibm,nmi-interlock" call. In such cases, as per
PAPR, subsequent processors serialize waiting for the first
processor to issue the "ibm,nmi-interlock" call. The second
processor that also received a machine check error waits
till the first processor is done reading the error log.
The first processor issues "ibm,nmi-interlock" call
when the error log is consumed.

Signed-off-by: Aravinda Prasad 
---
 hw/ppc/spapr.c |9 
 hw/ppc/spapr_rtas.c|   57 
 include/hw/ppc/spapr.h |5 +++-
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 9f2e5d2..6992b32 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2941,6 +2941,15 @@ static void spapr_machine_init(MachineState *machine)
 
 /* Resize rtas blob to accommodate error log */
 spapr->rtas_size = RTAS_ERROR_LOG_MAX;
+
+/* Set fwnmi capability in KVM */
+if (kvmppc_set_fwnmi() < 0) {
+error_report("Could not enable FWNMI capability");
+exit(1);
+}
+
+/* Register ibm,nmi-register and ibm,nmi-interlock RTAS calls */
+spapr_fwnmi_register();
 }
 
 spapr->rtas_blob = g_malloc(spapr->rtas_size);
diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index d8fb8a8..b569538 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -400,6 +400,55 @@ static void rtas_get_power_level(PowerPCCPU *cpu, 
SpaprMachineState *spapr,
 rtas_st(rets, 1, 100);
 }
 
+static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
+  SpaprMachineState *spapr,
+  uint32_t token, uint32_t nargs,
+  target_ulong args,
+  uint32_t nret, target_ulong rets)
+{
+hwaddr rtas_addr = spapr_get_rtas_addr();
+
+if (!rtas_addr) {
+rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+return;
+}
+
+if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_OFF) {
+rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
+return;
+}
+
+spapr->guest_machine_check_addr = rtas_ld(args, 1);
+rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
+static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
+   SpaprMachineState *spapr,
+   uint32_t token, uint32_t nargs,
+   target_ulong args,
+   uint32_t nret, target_ulong rets)
+{
+if (spapr->guest_machine_check_addr == -1) {
+/* NMI register not called */
+rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+return;
+}
+
+if (spapr->mc_status != cpu->vcpu_id) {
+/* The vCPU that hit the NMI should invoke "ibm,nmi-interlock" */
+rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
+return;
+}
+
+/*
+ * vCPU issuing "ibm,nmi-interlock" is done with NMI handling,
+ * hence unset mc_status.
+ */
+spapr->mc_status = -1;
+qemu_cond_signal(&spapr->mc_delivery_cond);
+rtas_st(rets, 0, RTAS_OUT_SUCCESS);
+}
+
 static struct rtas_call {
 const char *name;
 spapr_rtas_fn fn;
@@ -544,6 +593,14 @@ hwaddr spapr_get_rtas_addr(void)
 return (hwaddr)fdt32_to_cpu(*rtas_data);
 }
 
+void spapr_fwnmi_register(void)
+{
+spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
+rtas_ibm_nmi_register);
+spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
+rtas_ibm_nmi_interlock);
+}
+
 static void core_rtas_register_types(void)
 {
 spapr_rtas_register(RTAS_DISPLAY_CHARACTER, "display-character",
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index ffefde7..dada821 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -655,8 +655,10 @@ target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong 
opcode,
 #define RTAS_IBM_REMOVE_PE_DMA_WINDOW   (RTAS_TOKEN_BASE + 0x28)
 #define RTAS_IBM_RESET_PE_DMA_WINDOW(RTAS_TOKEN_BASE + 0x29)
 #define RTAS_IBM_SUSPEND_ME (RTAS_TOKEN_BASE + 0x2A)
+#define RTAS_IBM_NMI_REGISTER   (RTAS_TOKEN_BASE + 0x2B)
+#define RTAS_IBM_NMI_INTERLOCK  (RTAS_TOKEN_BASE + 0x2C)
 
-#define RTAS_TOKEN_MAX  (RTAS_TOKEN_BASE + 0x2B)
+#define RTAS_TOKEN_MAX  (RTAS_TOKEN_BASE + 0x2D)
 
 /* RTAS ibm,get-system-parameter token values */
 #define RTAS_SYSPARM_SPLPAR_CHARACTERISTICS  20
@@ -908,4 +910,5 @@ void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr 
pagesize,
 
 void spap

Re: [Qemu-devel] [RFC 0/3] Move notdirty handling to cputlb

[Paolo Bonzini]

On 18/09/19 07:26, Richard Henderson wrote:
> RFC because this doesn't work, and I don't quite understand why.
> The only failing test is {i386,x86_64} pxe-test -- the other
> migration tests that use notdirty all pass.
> 
> Note that if you try to reproduce this on x86, you'll likely
> have to --disable-kvm, as otherwise the pxe-test will skip tcg.
> 
> Anyone who knows how this works willing to have a look?

I think patch 2 is doing too much.  Why don't you keep
memory_notdirty_write_prepare and memory_notdirty_write_complete at
first (so that the atomic path is completely unchanged wrt MMU lookup),
and then simplify that separately?  Maybe that shows what's going on.

Paolo

[Qemu-devel] [PATCH v14 7/7] ppc: spapr: Activate the FWNMI functionality

[Aravinda Prasad]

This patch sets the default value of SPAPR_CAP_FWNMI_MCE
to SPAPR_CAP_ON for machine type 4.2.

Signed-off-by: Aravinda Prasad 
---
 hw/ppc/spapr.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a72a4b1..b6a249f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4580,7 +4580,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
void *data)
 smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF;
 smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON;
 smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF;
-smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF;
+smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_ON;
 spapr_caps_add_properties(smc, &error_abort);
 smc->irq = &spapr_irq_dual;
 smc->dr_phb_enabled = true;
@@ -4654,6 +4654,7 @@ static void spapr_machine_4_1_class_options(MachineClass 
*mc)
 smc->linux_pci_probe = false;
 compat_props_add(mc->compat_props, hw_compat_4_1, hw_compat_4_1_len);
 compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat));
+smc->default_caps.caps[SPAPR_CAP_FWNMI_MCE] = SPAPR_CAP_OFF;
 }
 
 DEFINE_SPAPR_MACHINE(4_1, "4.1", false);

[Qemu-devel] [PATCH v14 6/7] migration: Include migration support for machine check handling

[Aravinda Prasad]

This patch includes migration support for machine check
handling. Especially this patch blocks VM migration
requests until the machine check error handling is
complete as these errors are specific to the source
hardware and is irrelevant on the target hardware.

Signed-off-by: Aravinda Prasad 
---
 hw/ppc/spapr.c |   63 
 hw/ppc/spapr_events.c  |   16 +++-
 hw/ppc/spapr_rtas.c|2 ++
 include/hw/ppc/spapr.h |2 ++
 4 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6992b32..a72a4b1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -46,6 +46,7 @@
 #include "migration/qemu-file-types.h"
 #include "migration/global_state.h"
 #include "migration/register.h"
+#include "migration/blocker.h"
 #include "mmu-hash64.h"
 #include "mmu-book3s-v3.h"
 #include "cpu-models.h"
@@ -1829,6 +1830,8 @@ static void spapr_machine_reset(MachineState *machine)
 
 /* Signal all vCPUs waiting on this condition */
 qemu_cond_broadcast(&spapr->mc_delivery_cond);
+
+migrate_del_blocker(spapr->fwnmi_migration_blocker);
 }
 
 static void spapr_create_nvram(SpaprMachineState *spapr)
@@ -2119,6 +2122,60 @@ static const VMStateDescription vmstate_spapr_dtb = {
 },
 };
 
+static bool spapr_fwnmi_needed(void *opaque)
+{
+SpaprMachineState *spapr = (SpaprMachineState *)opaque;
+
+return spapr->guest_machine_check_addr != -1;
+}
+
+static int spapr_fwnmi_post_load(void *opaque, int version_id)
+{
+SpaprMachineState *spapr = (SpaprMachineState *)opaque;
+
+if (spapr_get_cap(spapr, SPAPR_CAP_FWNMI_MCE) == SPAPR_CAP_ON) {
+
+if (kvmppc_has_cap_ppc_fwnmi()) {
+return 0;
+}
+
+return kvmppc_set_fwnmi();
+}
+
+return 0;
+}
+
+static int spapr_fwnmi_pre_save(void *opaque)
+{
+SpaprMachineState *spapr = (SpaprMachineState *)opaque;
+
+/*
+ * With -only-migratable QEMU option, we cannot block migration.
+ * Hence check if machine check handling is in progress and print
+ * a warning message.
+ */
+if (spapr->mc_status != -1) {
+warn_report("A machine check is being handled during migration. The"
+"handler may run and log hardware error on the destination");
+}
+
+return 0;
+}
+
+static const VMStateDescription vmstate_spapr_machine_check = {
+.name = "spapr_machine_check",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = spapr_fwnmi_needed,
+.post_load = spapr_fwnmi_post_load,
+.pre_save = spapr_fwnmi_pre_save,
+.fields = (VMStateField[]) {
+VMSTATE_UINT64(guest_machine_check_addr, SpaprMachineState),
+VMSTATE_INT32(mc_status, SpaprMachineState),
+VMSTATE_END_OF_LIST()
+},
+};
+
 static const VMStateDescription vmstate_spapr = {
 .name = "spapr",
 .version_id = 3,
@@ -2152,6 +2209,7 @@ static const VMStateDescription vmstate_spapr = {
 &vmstate_spapr_dtb,
 &vmstate_spapr_cap_large_decr,
 &vmstate_spapr_cap_ccf_assist,
+&vmstate_spapr_machine_check,
 NULL
 }
 };
@@ -2948,6 +3006,11 @@ static void spapr_machine_init(MachineState *machine)
 exit(1);
 }
 
+/* Create the error string for live migration blocker */
+error_setg(&spapr->fwnmi_migration_blocker,
+"A machine check is being handled during migration. The handler"
+"may run and log hardware error on the destination");
+
 /* Register ibm,nmi-register and ibm,nmi-interlock RTAS calls */
 spapr_fwnmi_register();
 }
diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index ecc3d68..71caa03 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -43,6 +43,7 @@
 #include "qemu/main-loop.h"
 #include "hw/ppc/spapr_ovec.h"
 #include 
+#include "migration/blocker.h"
 
 #define RTAS_LOG_VERSION_MASK   0xff00
 #define   RTAS_LOG_VERSION_60x0600
@@ -844,6 +845,8 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
 {
 SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
 CPUState *cs = CPU(cpu);
+int ret;
+Error *local_err = NULL;
 
 if (spapr->guest_machine_check_addr == -1) {
 /*
@@ -873,8 +876,19 @@ void spapr_mce_req_event(PowerPCCPU *cpu, bool recovered)
 return;
 }
 }
-spapr->mc_status = cpu->vcpu_id;
 
+ret = migrate_add_blocker(spapr->fwnmi_migration_blocker, &local_err);
+if (ret == -EBUSY) {
+/*
+ * We don't want to abort so we let the migration to continue.
+ * In a rare case, the machine check handler will run on the target.
+ * Though this is not preferable, it is better than aborting
+ * the migration or killing the VM.
+ */
+warn_report_err(local_err);
+}
+
+spapr->mc_status = cpu->vcpu_id;
 spapr_mce_dispatch_elog(cpu, recovered);
 }
 
diff --

[Qemu-devel] [PATCH v14 0/7] target-ppc/spapr: Add FWNMI support in QEMU for PowerKVM guests

[Aravinda Prasad]

This patch set adds support for FWNMI in PowerKVM guests.

System errors such as SLB multihit and memory errors
that cannot be corrected by hardware is passed on to
the kernel for handling by raising machine check
exception (an NMI). Upon such machine check exceptions,
if the address in error belongs to guest then KVM
invokes guests' 0x200 interrupt vector if the guest
is not FWNMI capable. For FWNMI capable guest
KVM passes the control to QEMU by exiting the guest.

This patch series adds functionality to QEMU to pass
on such machine check exceptions to the FWNMI capable
guest kernel by building an error log and invoking
the guest registered machine check handling routine.

The KVM changes are now part of the upstream kernel
(commit e20bbd3d). This series contain QEMU changes.

Change Log v14:
  - Feature activation moved to a separate patch
  - Fixed issues with migration blocker

Change Log v13:
  - Minor fixes (mostly nits)
  - Moved FWNMI guest registration check from patch 4 to 3.

Change Log v12:
  - Rebased to latest ppc-for-4.2 (SHA b1e8156743)

Change Log v11:
  - Moved FWNMI SPAPR cap defaults to 4.2 class option
  - Fixed issues with handling fwnmi KVM capability

Change Log v10:
  - Reshuffled the patch sequence + minor fixes

Change Log v9:
  - Fixed kvm cap and spapr cap issues

Change Log v8:
  - Added functionality to check FWNMI capability during
VM migration

---

Aravinda Prasad (7):
  Wrapper function to wait on condition for the main loop mutex
  ppc: spapr: Introduce FWNMI capability
  target/ppc: Handle NMI guest exit
  target/ppc: Build rtas error log upon an MCE
  ppc: spapr: Handle "ibm,nmi-register" and "ibm,nmi-interlock" RTAS calls
  migration: Include migration support for machine check handling
  ppc: spapr: Activate the FWNMI functionality


 cpus.c   |5 +
 hw/ppc/spapr.c   |   95 
 hw/ppc/spapr_caps.c  |   29 +
 hw/ppc/spapr_events.c|  271 ++
 hw/ppc/spapr_rtas.c  |   85 ++
 include/hw/ppc/spapr.h   |   25 
 include/qemu/main-loop.h |8 +
 target/ppc/kvm.c |   42 +++
 target/ppc/kvm_ppc.h |   14 ++
 target/ppc/trace-events  |1 
 10 files changed, 573 insertions(+), 2 deletions(-)

--
Aravinda Prasad

Re: [Qemu-devel] [PATCH] * include/fpu/softfloat.h (floatx80_invalid_encoding): Handle m68k specific infinity pattern.

[Pierre Muller]

  Hi Thomas,

  I tried to use git format-patch -s below,
and change the commit message that appears below:


muller@gcc123:~/gnu/qemu/qemu$ git format-patch -s 
a017dc6d43aaa4ffc7be40ae3adee4086be9cec2^
0001-Fix-floatx80_invalid_encoding-function-for-m68k-cpu.patch
muller@gcc123:~/gnu/qemu/qemu$ cat 
0001-Fix-floatx80_invalid_encoding-function-for-m68k-cpu.patch
>From a017dc6d43aaa4ffc7be40ae3adee4086be9cec2 Mon Sep 17 00:00:00 2001
From: Pierre Muller 
Date: Wed, 18 Sep 2019 08:04:19 +
Subject: [PATCH]Fix floatx80_invalid_encoding function for m68k cpu

As m68k accepts different patterns for infinity,
and additional test for valid infinity must be added
for m68k cpu target.

Signed-off-by: Pierre Muller 
---
 include/fpu/softfloat.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index ecb8ba0114..dea24384e9 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -685,10 +685,17 @@ static inline int floatx80_is_any_nan(floatx80 a)
 | pseudo-infinities and un-normal numbers. It does not include
 | pseudo-denormals, which must still be correctly handled as inputs even
 | if they are never generated as outputs.
+| As m68k accepts different patterns for infinity, thus an additional test
+| for valid infinity value must be added for m68k CPU.
 **/
 static inline bool floatx80_invalid_encoding(floatx80 a)
 {
+#if defined (TARGET_M68K)
+return ((a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0)
+   && (! floatx80_is_infinity(a));
+#else
 return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
+#endif
 }

 #define floatx80_zero make_floatx80(0x, 0xLL)
--
2.20.1

I hope this is correct according to the guidelines.


Le 18/09/2019 à 09:26, Thomas Huth a écrit :
> On 17/09/2019 22.04, Pierre Muller wrote:
>>
>> Hello,

> 
>  Hi Pierre,
> 
> thanks a lot for the patch! But please note that the QEMU project has
> some constraints for patches that have to be followed before a patch can
> be applied.
> 
> Most important one: You need to provide a "Signed-off-by:" line in the
> patch description to make sure that you agree with the Developer
> Certificate Of Origin. See this URL for more details:
> 
>  https://wiki.qemu.org/Contribute/SubmitAPatch

   I tried to follow this guide.

> Then it would be nice if you add some proper commit message to the patch
> (something similar to the comment that you've added to the source code
> would do the job, I guess).

  I hope the above is OK.

> And finally, please note that qemu-devel is a high traffic mailing list.
> When sending patches, you best make sure to put some maintainers on CC:,
> or your patch might get lost in the high traffic. You can either have a
> look at the MAINTAINERS file in the main directory, or use the
> scripts/get_maintainers.pl script on your patch to see who should be put
> on CC:.

Thus I took the liberty to use 'Reply to all' as you have
added several persons which are listed using
./scripts/get_maintainer.pl -f  include/fpu/softfloat.h
and Laurent who is more specifically involved in m68k support.

Please let me know if more is needed to get this patch accepted.

Pierre Muller

Re: [Qemu-devel] [PATCH 2/2] qapi: Allow introspecting fix for savevm's cooperation with blockdev

[Kevin Wolf]

Am 17.09.2019 um 18:33 hat Eric Blake geschrieben:
> On 9/17/19 10:49 AM, Peter Krempa wrote:
> > savevm was buggy as it considered all monitor owned block device nodes
> > for snapshot. With introduction of -blockdev the common usage made all
> > nodes including protocol nodes monitor owned and thus considered for
> > snapshot. This was fixed but clients need to be able to detect whether
> > this fix is present.
> > 
> > Since savevm does not have an QMP alternative add the feature for the
> > 'human-monitor-command' backdoor which is used to call this command in
> > modern use.
> > 
> > Signed-off-by: Peter Krempa 
> > ---
> >  qapi/misc.json | 8 +++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi/misc.json b/qapi/misc.json
> > index 6bd11f50e6..e2b33c3f8a 100644
> > --- a/qapi/misc.json
> > +++ b/qapi/misc.json
> > @@ -1020,6 +1020,11 @@
> >  #
> >  # @cpu-index: The CPU to use for commands that require an implicit CPU
> >  #
> > +# Features:
> > +# @savevm-blockdev-monitor-nodes: If present, the savevm monitor command
> > +# correctly handles monitor owned block 
> > nodes
> > +# when taking a snapshot.
> 
> Is it worth adding a '(since 4.2)' on when features are added?

I would also rather describe the change in behaviour ("only includes
monitor owned block nodes if they have no parents") than saying that the
behaviour is now correct.

(Not the least because we know that the current way still isn't quite
correct, just hopefully correct enough: Block job BlockBackends are
currently snapshotted, which is unintended and will be changed in the
future. However, in practice people probably won't use block jobs on
non-root nodes and internal snapshots together.)

> > +#
> >  # Returns: the output of the command as a string
> >  #
> >  # Since: 0.14.0
> > @@ -1047,7 +1052,8 @@
> >  ##
> >  { 'command': 'human-monitor-command',
> >'data': {'command-line': 'str', '*cpu-index': 'int'},
> > -  'returns': 'str' }
> > +  'returns': 'str',
> > +  'features' : [ { 'name': 'savevm-blockdev-monitor-nodes' } ] }
> 
> We could, of course, actually implement a QMP 'savevm' and use _that_ as
> the introspection.  But that's a bigger can of worms, so this is
> reasonable enough for the 4.2 timeframe.

I think a QMP savevm would sidestep the whole issue by taking an
explicit list of nodes to snapshot, and an explicit option to tell which
node to store the vmstate on.

Kevin


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 00/29] s390x/tcg: mem_helper: Fault-safe handling

[David Hildenbrand]

On 16.09.19 15:57, David Hildenbrand wrote:
> This series fixes a bunch of issues related to some mem helpers and makes
> sure that they are fault-safe, meaning no system state is modified in case
> a fault is triggered.
> 
> I can spot tons of other issues with other mem helpers that will have
> to be fixed later. Also, fault-safe handling for some instructions
> (especially TR) might be harder to implement (you don't know what will
> actually be accessed upfront - we might need a buffer and go over
> inputs twice). Focusing on the MOVE instructions for now.
> 
> 
> 
> Newer versions of glibc use memcpy() in memmove() for forward moves. The
> implementation makese use of MVC. The TCG implementation of MVC is
> currently not able to handle faults reliably when crossing pages. MVC
> can cross with 256 bytes at most two pages.
> 
> In case we get a fault on the second page, we already moved data. When
> continuing after the fault we might try to move already overwritten data,
> which is very bad in case we have overlapping data on a forward move.
> 
> Triggered for now only by rpmbuild (crashes when checking the spec file)
> and rpm (database corruptions). This fixes installing Fedora rawhide (31)
> under TCG.
> 
> This was horrible to debug as it barely triggers and we fail at completely
> different places.
> 
> Cc: Stefano Brivio 
> Cc: Florian Weimer 
> Cc: Dan Horák 
> Cc: Cole Robinson 
> 
> v2 -> v3:
> - "s390x/tcg: MVCL: Zero out unused bits of address"
> -- Do single deposit for 24/31-bit
> - "s390x/tcg: MVCL: Process max 4k bytes at a time"
> -- Use max of 4k instead of 2k, limiting to single pages
> - "s390x/tcg: MVCLU/MVCLE: Process max 4k bytes at a time"
> -- Limit to single pages
> - "s390x/tcg: MVCOS: Lengths are 32 bit in 24/31-bit mode"
> -- Added
> - "s390x/tcg: MVCS/MVCP: Properly wrap the length"
> -- Properly use 32 instead of 31 bit.
> - "s390x/tcg: MVST: Fix storing back the addresses to registers"
> -- Read R0 implicitly
> - "s390x/tcg: Fault-safe memset"
> -- Speed up TLB_NOTDIRTY handling
> -- Move single-page access to helper function
> -- Pass access structure to access_memset()
> -- Replace access_prepare() by previous access_prepare_idx()
> - "s390x/tcg: Fault-safe memmove"
> -- Pass access structure to access_memmove()
> -- Speed up TLB_NOTDIRTY handling when accessing single bytes
> - The other fault-safe handling patches were adapted to work with the
>   changed access functions. mmu_idx is now always passed to
>   access_prepare() from the helpers.
> 
> v1 -> v2:
> - Include many fixes
> - Fix more instructions
> - Use the new probe_access() function
> - Include "tests/tcg: target/s390x: Test MVO"
> 
> David Hildenbrand (29):
>   s390x/tcg: Reset exception_index to -1 instead of 0
>   s390x/tcg: MVCL: Zero out unused bits of address
>   s390x/tcg: MVCL: Detect destructive overlaps
>   s390x/tcg: MVCL: Process max 4k bytes at a time
>   s390x/tcg: MVC: Increment the length once
>   s390x/tcg: MVC: Use is_destructive_overlap()
>   s390x/tcg: MVPG: Check for specification exceptions
>   s390x/tcg: MVPG: Properly wrap the addresses
>   s390x/tcg: MVCLU/MVCLE: Process max 4k bytes at a time
>   s390x/tcg: MVCS/MVCP: Check for special operation exceptions
>   s390x/tcg: MVCOS: Lengths are 32 bit in 24/31-bit mode
>   s390x/tcg: MVCS/MVCP: Properly wrap the length
>   s390x/tcg: MVST: Check for specification exceptions
>   s390x/tcg: MVST: Fix storing back the addresses to registers
>   s390x/tcg: Always use MMU_USER_IDX for CONFIG_USER_ONLY
>   s390x/tcg: Fault-safe memset
>   s390x/tcg: Fault-safe memmove
>   s390x/tcg: MVCS/MVCP: Use access_memmove()
>   s390x/tcg: MVC: Fault-safe handling on destructive overlaps
>   s390x/tcg: MVCLU: Fault-safe handling
>   s390x/tcg: OC: Fault-safe handling
>   s390x/tcg: XC: Fault-safe handling
>   s390x/tcg: NC: Fault-safe handling
>   s390x/tcg: MVCIN: Fault-safe handling
>   s390x/tcg: MVN: Fault-safe handling
>   s390x/tcg: MVZ: Fault-safe handling
>   s390x/tcg: MVST: Fault-safe handling
>   s390x/tcg: MVO: Fault-safe handling
>   tests/tcg: target/s390x: Test MVO
> 
>  target/s390x/cpu.h  |   4 +
>  target/s390x/helper.h   |   2 +-
>  target/s390x/insn-data.def  |   2 +-
>  target/s390x/mem_helper.c   | 743 ++--
>  target/s390x/translate.c|  12 +-
>  tests/tcg/s390x/Makefile.target |   1 +
>  tests/tcg/s390x/mvo.c   |  25 ++
>  7 files changed, 564 insertions(+), 225 deletions(-)
>  create mode 100644 tests/tcg/s390x/mvo.c
> 

As long as there are no further comments, this series is ready to go
(only one patch description needs a fixup).

Conny, how do you prefer to upstream this stuff? (remembering that
you'll be on vacation soon).

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [for-4.2 PATCH 0/6] Block-related record/replay fixes

[Pavel Dovgalyuk]

> -Original Message-
> From: Alex Bennée [mailto:alex.ben...@linaro.org]
> Sent: Tuesday, September 17, 2019 10:02 PM
> To: Pavel Dovgalyuk
> Cc: qemu-devel@nongnu.org; kw...@redhat.com; peter.mayd...@linaro.org;
> crosthwaite.pe...@gmail.com; boost.li...@gmail.com; 
> artem.k.pisare...@gmail.com;
> quint...@redhat.com; ciro.santi...@gmail.com; jasow...@redhat.com; 
> m...@redhat.com;
> arm...@redhat.com; mre...@redhat.com; maria.klimushenk...@ispras.ru; 
> dovga...@ispras.ru;
> kra...@redhat.com; pavel.dovga...@ispras.ru; thomas.dull...@googlemail.com;
> pbonz...@redhat.com; dgilb...@redhat.com; r...@twiddle.net
> Subject: Re: [for-4.2 PATCH 0/6] Block-related record/replay fixes
> 
> 
> Pavel Dovgalyuk  writes:
> 
> > The set of patches include the block-related updates
> > of the record/replay icount feature:
> >  - application of 'snapshot' option on the file layer instead of
> >the top one: command line and documentation fix
> >  - implementation of bdrv_snapshot_goto for blkreplay driver
> >  - start/stop fix in replay mode with attached block devices
> >  - record/replay of bh oneshot events, used in the block layer
> 
> Can we come up with a reasonable smoke test to verify record/replay is
> functional? AIUI we don't need a full OS but we do need a block device
> to store the replay data. Could we use one of the simple system tests
> like "memory" and run that through a record and replay cycle?

That's would be great.
I'm not familiar with testing framework and couldn't find "memory" test that 
you refer to.
Replay test must at least the following:
1. record some execution
2. replay that execution

And there could be several endings:
1. record stalled
2. record crashed
3. replay stalled
4. replay crashed
5. all executions finished successfully

Pavel Dovgalyuk

> 
> >
> > ---
> >
> > Pavel Dovgaluk (6):
> >   block: implement bdrv_snapshot_goto for blkreplay
> >   replay: disable default snapshot for record/replay
> >   replay: update docs for record/replay with block devices
> >   replay: don't drain/flush bdrv queue while RR is working
> >   replay: finish record/replay before closing the disks
> >   replay: add BH oneshot event for block layer
> >
> >
> >  block/blkreplay.c|8 
> >  block/block-backend.c|9 ++---
> >  block/io.c   |   32 ++--
> >  block/iscsi.c|5 +++--
> >  block/nfs.c  |6 --
> >  block/null.c |4 +++-
> >  block/nvme.c |6 --
> >  block/rbd.c  |5 +++--
> >  block/vxhs.c |5 +++--
> >  cpus.c   |2 --
> >  docs/replay.txt  |   12 +---
> >  include/sysemu/replay.h  |4 
> >  replay/replay-events.c   |   16 
> >  replay/replay-internal.h |1 +
> >  replay/replay.c  |2 ++
> >  stubs/Makefile.objs  |1 +
> >  stubs/replay-user.c  |9 +
> >  vl.c |   11 +--
> >  18 files changed, 115 insertions(+), 23 deletions(-)
> >  create mode 100644 stubs/replay-user.c
> 
> 
> --
> Alex Bennée

Re: [Qemu-devel] [libvirt] Call for volunteers: LWN.net articles about KVM Forum talks

[Stefano Garzarella]

On Tue, Sep 17, 2019 at 02:02:59PM +0100, Stefan Hajnoczi wrote:
> Hi,
> LWN.net is a popular open source news site that covers Linux and other
> open source communities (Python, GNOME, Debian, etc).  It has published
> a few KVM articles in the past too.
> 
> Let's raise awareness of QEMU, KVM, and libvirt by submitting articles 
> covering
> KVM Forum.
> 
> I am looking for ~5 volunteers who are attending KVM Forum to write an article
> about a talk they find interesting.
> 
> Please pick a talk you'd like to cover and reply to this email thread.
> I will then send an email to LWN with a heads-up so they can let us know
> if they are interested in publishing a KVM Forum special.  I will not
> ask LWN.net for money.
> 
> KVM Forum schedule:
> https://events.linuxfoundation.org/events/kvm-forum-2019/program/schedule/
> 
> LWN.net guidelines:
> https://lwn.net/op/AuthorGuide.lwn
> "Our general guideline is for articles to be around 1500 words in
> length, though somewhat longer or shorter can work too. The best
> articles cover a fairly narrow topic completely, without any big
> omissions or any extra padding."
> 
> I volunteer to cover Michael Tsirkin's "VirtIO without the Virt -
> Towards Implementations in Hardware" talk.

I volunteer for "Libvirt: Never too Late to Learn New Tricks" by
Daniel Berrange.

Cheers,
Stefano

Re: [Qemu-devel] [RFC 0/3] Move notdirty handling to cputlb

[David Hildenbrand]

On 18.09.19 07:26, Richard Henderson wrote:
> RFC because this doesn't work, and I don't quite understand why.
> The only failing test is {i386,x86_64} pxe-test -- the other
> migration tests that use notdirty all pass.
> 
> Note that if you try to reproduce this on x86, you'll likely
> have to --disable-kvm, as otherwise the pxe-test will skip tcg.
> 
> Anyone who knows how this works willing to have a look?
> 
> 
> r~
> 
> 
> Richard Henderson (3):
>   exec: Adjust notdirty tracing
>   cputlb: Move NOTDIRTY handling from I/O path to TLB path
>   cputlb: Remove ATOMIC_MMU_DECLS
> 
>  accel/tcg/atomic_template.h| 12 -
>  include/exec/cpu-common.h  |  1 -
>  include/exec/memory-internal.h | 53 +++
>  accel/tcg/cputlb.c | 65 +--
>  accel/tcg/user-exec.c  |  1 -
>  exec.c | 97 +++---
>  memory.c   | 20 ---
>  trace-events   |  4 +-
>  8 files changed, 63 insertions(+), 190 deletions(-)
> 

Cool, this might speed up some accesses - and will definetly cleanup
quite some code.

Can you CC me on further iterations, so I can test on s390x? Thanks!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH 2/2] qapi: Allow introspecting fix for savevm's cooperation with blockdev

[Peter Krempa]

On Wed, Sep 18, 2019 at 10:22:13 +0200, Kevin Wolf wrote:
> Am 17.09.2019 um 18:33 hat Eric Blake geschrieben:
> > On 9/17/19 10:49 AM, Peter Krempa wrote:
> > > savevm was buggy as it considered all monitor owned block device nodes
> > > for snapshot. With introduction of -blockdev the common usage made all
> > > nodes including protocol nodes monitor owned and thus considered for
> > > snapshot. This was fixed but clients need to be able to detect whether
> > > this fix is present.
> > > 
> > > Since savevm does not have an QMP alternative add the feature for the
> > > 'human-monitor-command' backdoor which is used to call this command in
> > > modern use.
> > > 
> > > Signed-off-by: Peter Krempa 
> > > ---
> > >  qapi/misc.json | 8 +++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/qapi/misc.json b/qapi/misc.json
> > > index 6bd11f50e6..e2b33c3f8a 100644
> > > --- a/qapi/misc.json
> > > +++ b/qapi/misc.json
> > > @@ -1020,6 +1020,11 @@
> > >  #
> > >  # @cpu-index: The CPU to use for commands that require an implicit CPU
> > >  #
> > > +# Features:
> > > +# @savevm-blockdev-monitor-nodes: If present, the savevm monitor command
> > > +# correctly handles monitor owned block 
> > > nodes
> > > +# when taking a snapshot.
> > 
> > Is it worth adding a '(since 4.2)' on when features are added?
> 
> I would also rather describe the change in behaviour ("only includes
> monitor owned block nodes if they have no parents") than saying that the
> behaviour is now correct.

That's a good idea. I'll post it in a v2 if required.

> (Not the least because we know that the current way still isn't quite
> correct, just hopefully correct enough: Block job BlockBackends are
> currently snapshotted, which is unintended and will be changed in the
> future. However, in practice people probably won't use block jobs on
> non-root nodes and internal snapshots together.)
> 
> > > +#
> > >  # Returns: the output of the command as a string
> > >  #
> > >  # Since: 0.14.0
> > > @@ -1047,7 +1052,8 @@
> > >  ##
> > >  { 'command': 'human-monitor-command',
> > >'data': {'command-line': 'str', '*cpu-index': 'int'},
> > > -  'returns': 'str' }
> > > +  'returns': 'str',
> > > +  'features' : [ { 'name': 'savevm-blockdev-monitor-nodes' } ] }
> > 
> > We could, of course, actually implement a QMP 'savevm' and use _that_ as
> > the introspection.  But that's a bigger can of worms, so this is
> > reasonable enough for the 4.2 timeframe.
> 
> I think a QMP savevm would sidestep the whole issue by taking an
> explicit list of nodes to snapshot, and an explicit option to tell which
> node to store the vmstate on.

A straight replacement for savevm would be quite pointless and also such
discussion took already place multiple times in the past. The result
always was that we need something better than just a qmp version of
savevm.

I'll be happy to implement the libvirt support for the "new" savevm if
it ever appears.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [RFC 1/3] exec: Adjust notdirty tracing

[David Hildenbrand]

On 18.09.19 07:26, Richard Henderson wrote:
> The memory_region_tb_read tracepoint is unreachable, since notdirty
> is supposed to apply only to reads.  The memory_region_tb_write
> tracepoint is mis-named, because notdirty is not only used for TB
> invalidation.  It is also used for e.g. VGA RAM updates.
> 
> Replace memory_region_tb_write with memory_notdirty_write, and
> place it in memory_notdirty_write_prepare where it can catch all
> of the instances.  Add memory_notdirty_dirty to log when we no
> longer intercept writes to a page.
> 
> Signed-off-by: Richard Henderson 
> ---
>  exec.c   | 3 +++
>  memory.c | 4 
>  trace-events | 4 ++--
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 8b998974f8..9babe57615 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2755,6 +2755,8 @@ void memory_notdirty_write_prepare(NotDirtyInfo *ndi,
>  ndi->size = size;
>  ndi->pages = NULL;
>  
> +trace_memory_notdirty_write(mem_vaddr, ram_addr, size);
> +
>  assert(tcg_enabled());
>  if (!cpu_physical_memory_get_dirty_flag(ram_addr, DIRTY_MEMORY_CODE)) {
>  ndi->pages = page_collection_lock(ram_addr, ram_addr + size);
> @@ -2779,6 +2781,7 @@ void memory_notdirty_write_complete(NotDirtyInfo *ndi)
>  /* we remove the notdirty callback only if the code has been
> flushed */
>  if (!cpu_physical_memory_is_clean(ndi->ram_addr)) {
> +trace_memory_notdirty_dirty(ndi->mem_vaddr);
>  tlb_set_dirty(ndi->cpu, ndi->mem_vaddr);
>  }
>  }
> diff --git a/memory.c b/memory.c
> index b9dd6b94ca..57c44c97db 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -438,7 +438,6 @@ static MemTxResult  
> memory_region_read_accessor(MemoryRegion *mr,
>  /* Accesses to code which has previously been translated into a TB 
> show
>   * up in the MMIO path, as accesses to the io_mem_notdirty
>   * MemoryRegion. */
> -trace_memory_region_tb_read(get_cpu_index(), addr, tmp, size);
>  } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
>  hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
>  trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, 
> size);
> @@ -465,7 +464,6 @@ static MemTxResult 
> memory_region_read_with_attrs_accessor(MemoryRegion *mr,
>  /* Accesses to code which has previously been translated into a TB 
> show
>   * up in the MMIO path, as accesses to the io_mem_notdirty
>   * MemoryRegion. */
> -trace_memory_region_tb_read(get_cpu_index(), addr, tmp, size);
>  } else if (TRACE_MEMORY_REGION_OPS_READ_ENABLED) {
>  hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
>  trace_memory_region_ops_read(get_cpu_index(), mr, abs_addr, tmp, 
> size);
> @@ -490,7 +488,6 @@ static MemTxResult 
> memory_region_write_accessor(MemoryRegion *mr,
>  /* Accesses to code which has previously been translated into a TB 
> show
>   * up in the MMIO path, as accesses to the io_mem_notdirty
>   * MemoryRegion. */
> -trace_memory_region_tb_write(get_cpu_index(), addr, tmp, size);
>  } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
>  hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
>  trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, 
> size);
> @@ -515,7 +512,6 @@ static MemTxResult 
> memory_region_write_with_attrs_accessor(MemoryRegion *mr,
>  /* Accesses to code which has previously been translated into a TB 
> show
>   * up in the MMIO path, as accesses to the io_mem_notdirty
>   * MemoryRegion. */
> -trace_memory_region_tb_write(get_cpu_index(), addr, tmp, size);
>  } else if (TRACE_MEMORY_REGION_OPS_WRITE_ENABLED) {
>  hwaddr abs_addr = memory_region_to_absolute_addr(mr, addr);
>  trace_memory_region_ops_write(get_cpu_index(), mr, abs_addr, tmp, 
> size);
> diff --git a/trace-events b/trace-events
> index 823a4ae64e..5c9a1631e7 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -52,14 +52,14 @@ dma_map_wait(void *dbs) "dbs=%p"
>  find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%" PRIx64 " @ 0x%" 
> PRIx64
>  find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset, 
> uint64_t next, uint64_t mingap) "trying size: 0x%" PRIx64 " @ 0x%" PRIx64 ", 
> offset: 0x%" PRIx64" next: 0x%" PRIx64 " mingap: 0x%" PRIx64
>  ram_block_discard_range(const char *rbname, void *hva, size_t length, bool 
> need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d 
> fallocate: %d ret: %d"
> +memory_notdirty_write(uint64_t vaddr, uint64_t ram_addr, unsigned size) 
> "0x%" PRIx64 " ram_addr 0x%" PRIx64 " size %u"
> +memory_notdirty_dirty(uint64_t vaddr) "0x%" PRIx64

My only suggestion would be to give slightly better names like

memory_notdirty_write_access()
memory_notdirty_set_dirty()

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] vhost, iova, and dirty page tracking

[Tian, Kevin]

> From: Jason Wang [mailto:jasow...@redhat.com]
> Sent: Wednesday, September 18, 2019 2:10 PM
> 
> >>
> >> Note that the HVA to GPA mapping is not an 1:1 mapping. One HVA
> range
> >> could be mapped to several GPA ranges.
> > This is fine. Currently vfio_dma maintains IOVA->HVA mapping.
> >
> > btw under what condition HVA->GPA is not 1:1 mapping? I didn't realize it.
> 
> 
> I don't remember the details e.g memory region alias? And neither kvm
> nor kvm API does forbid this if my memory is correct.
> 

I checked https://qemu.weilnetz.de/doc/devel/memory.html, which
provides an example of aliased layout. However, its aliasing is all
1:1, instead of N:1. From guest p.o.v every writable GPA implies an
unique location. Why would we hit the situation where multiple
write-able GPAs are mapped to the same HVA (i.e. same physical
memory location)? Is Qemu doing its own same-content memory
merging in GPA level, similar to KSM?

Thanks
Kevin

Re: [Qemu-devel] [PATCH] vhost-user: save features if the char dev is closed

[Adrian Moreno]

On 9/17/19 11:40 PM, Michael S. Tsirkin wrote:
> On Tue, Sep 17, 2019 at 09:19:01PM +0200, Adrian Moreno wrote:
>> That way the state can be correctly restored when the device is opened
>> again. This might happen if the backend is restarted.
>>
>> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1738768
>> Reported-by: Pei Zhang 
>> Fixes: 6ab79a20af3a (do not call vhost_net_cleanup() on running net from 
>> char user event)
>> Cc: ddstr...@canonical.com
>> Cc: Michael S. Tsirkin 
>>
>> Signed-off-by: Adrian Moreno 
>> ---
>>  net/vhost-user.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/net/vhost-user.c b/net/vhost-user.c
>> index 51921de443..acf20cb9e0 100644
>> --- a/net/vhost-user.c
>> +++ b/net/vhost-user.c
>> @@ -235,6 +235,13 @@ static void chr_closed_bh(void *opaque)
>>  
>>  s = DO_UPCAST(NetVhostUserState, nc, ncs[0]);
>>  
>> +if (s->vhost_net) {
>> +uint64_t features = vhost_net_get_acked_features(s->vhost_net);
>> +if (features) {
>> +s->acked_features = features;
>> + }
> 
> why does it make sense to check if (features)?
> 0x0 is a valid feature bitmap, isn't it?
You're right. It doesn't.
I'll remove the check.

> 
>> +}
>> +
>>  qmp_set_link(name, false, &err);
>>  
>>  qemu_chr_fe_set_handlers(&s->chr, NULL, NULL, net_vhost_user_event,
>> -- 
>> 2.21.0

Thanks.

[Qemu-devel] [PATCH v2] vhost-user: save features if the char dev is closed

[Adrian Moreno]

That way the state can be correctly restored when the device is opened
again. This might happen if the backend is restarted.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1738768
Reported-by: Pei Zhang 
Fixes: 6ab79a20af3a (do not call vhost_net_cleanup() on running net from char 
user event)
Cc: ddstr...@canonical.com
Cc: Michael S. Tsirkin 

Signed-off-by: Adrian Moreno 
---
 net/vhost-user.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index 51921de443..014199d600 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -235,6 +235,10 @@ static void chr_closed_bh(void *opaque)
 
 s = DO_UPCAST(NetVhostUserState, nc, ncs[0]);
 
+if (s->vhost_net) {
+s->acked_features = vhost_net_get_acked_features(s->vhost_net);
+}
+
 qmp_set_link(name, false, &err);
 
 qemu_chr_fe_set_handlers(&s->chr, NULL, NULL, net_vhost_user_event,
-- 
2.21.0

Re: [Qemu-devel] [PATCH v3 00/18] ppc/pnv: add XIVE support for KVM guests

[Cédric Le Goater]

On 18/09/2019 07:44, David Gibson wrote:
> On Tue, Sep 17, 2019 at 01:54:24PM +0200, Cédric Le Goater wrote:
>> On 31/07/2019 16:12, Cédric Le Goater wrote:
>>> Hello,
>>>
>>> The QEMU PowerNV machine emulates a baremetal OpenPOWER system and
>>> acts as an hypervisor (L0). Supporting emulation of KVM to run guests
>>> (L1) requires a few more extensions, among which guest support for the
>>> XIVE interrupt controller on POWER9 processor.
>>>
>>> The following changes add new per-CPU PowerNV machines and extend the
>>> XIVE models with the new XiveFabric and XivePresenter interfaces to
>>> provide support for XIVE escalations and interrupt resend. This
>>> mechanism is used by XIVE to notify the hypervisor that a vCPU is not
>>> dispatched on a HW thread. Tested on a QEMU PowerNV machine and a
>>> simple QEMU pseries guest doing network on a local bridge.
>>>
>>> The XIVE interrupt controller offers a way to increase the XIVE
>>> resources per chip by configuring multiple XIVE blocks on a chip. This
>>> is not currently supported by the model. However, some configurations,
>>> such as OPAL/skiboot, use one block-per-chip configuration with some
>>> optimizations. One of them is to override the hardwired chip ID by the
>>> block id in the PowerBUS operations and for CAM line compares. This
>>> patchset improves the support for this setup. Tested with 4 chips.
>>
>> David,
>>
>> Do you want me to resend this patchset ? or you just didn't have time
>> to look at it ?  
> 
> Mostly, I just haven't had time.  I'm also finding the patches pretty
> difficult to read and review.  I don't think that's an indication
> they're bad, just that what they're doing is necessarily complex, but
> it's still made it hard to tackle them.

I will try to split the initial patches on the presenter a little more.


>> Patch 16 has changed a little since. The get_block_id() handler has 
>> moved to the XiveRouterClass.
> 
> You, might as well repost, so I'm looking at the latest stuff.  I
> can't promise I'll be able to look at the new set terribly soon
> though.

ok.

thanks,

C.

[Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Kevin Wolf]

Running iotests is not required to build QEMU, so we can have stricter
version requirements for Python here and can make use of new features
and drop compatibility code earlier.

This makes qemu-iotests skip all Python tests if a Python version before
3.5 is used for the build.

Suggested-by: Eduardo Habkost 
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/check | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 875399d79f..a68f414d6c 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -633,6 +633,13 @@ then
 export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
 fi
 
+# Note that if the Python conditional here evaluates True we will exit
+# with status 1 which is a shell 'false' value.
+python_usable=false
+if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; then
+python_usable=true
+fi
+
 default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
 default_alias_machine=$($QEMU_PROG -machine help | \
sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
@@ -809,7 +816,12 @@ do
 start=$(_wallclock)
 
 if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env python" 
]; then
-run_command="$PYTHON $seq"
+if $python_usable; then
+run_command="$PYTHON $seq"
+else
+run_command="false"
+echo "Unsupported Python version" > $seq.notrun
+fi
 else
 run_command="./$seq"
 fi
-- 
2.20.1

[Qemu-devel] qemu patches

[Ilias]

Hi,

Your recent patch

https://github.com/qemu/qemu/commit/12e9493df9242a2051701e7eb64175d4e904acba#diff-d98f9b48cc332d226892f0db74a86b87

to the file

target/i386/whpx-all.c


breaks compilation when WHPX is enabled.

Please add

#include "hw/boards.h"

to the #include section of the above file again to fix it.

Re: [Qemu-devel] [PATCH v4 02/10] hw/core: create Resettable QOM interface

[David Gibson]

On Wed, Sep 11, 2019 at 04:56:13PM +0200, Damien Hedde wrote:
> 
> 
> On 9/11/19 10:06 AM, David Gibson wrote:
> > On Wed, Aug 21, 2019 at 06:33:33PM +0200, Damien Hedde wrote:
> >> This commit defines an interface allowing multi-phase reset. This aims
> >> to solve a problem of the actual single-phase reset (built in
> >> DeviceClass and BusClass): reset behavior is dependent on the order
> >> in which reset handlers are called. In particular doing external
> >> side-effect (like setting an qemu_irq) is problematic because receiving
> >> object may not be reset yet.
> >>
> >> The Resettable interface divides the reset in 3 well defined phases.
> >> To reset an object tree, all 1st phases are executed then all 2nd then
> >> all 3rd. See the comments in include/hw/resettable.h for a more complete
> >> description. There is 3 phases to allow a device to be held in reset
> >> state; the ability to control this state will be added in a following
> >> commits.
> >>
> >> The qdev/qbus reset in DeviceClass and BusClass will be modified in
> >> following commits to use this interface.
> >> No change of behavior is expected because the init phase execution order
> >> follows the children-then-parent order inside a tree. Since this is the
> >> actual order of qdev/qbus reset, we will be able to map current reset
> >> handlers on init phase for example.
> >>
> >> In this patch only cold reset is introduced, which is pretty much the
> >> actual semantics of the current reset handlers. The interface can be
> >> extended to support more reset types.
> >>
> >> Documentation will be added in a following commit.
> >>
> >> Signed-off-by: Damien Hedde 
> >> ---
> >>
> >> I kept the non-recursive count approach (a given object counts reset
> >> initiated on it as well as reset initiated on its parents in reset
> >> hierarchy). I implemented the other approach, it is possible but is more
> >> complex (an object has to know its direct parent(s) and we need to scan
> >> the reset hierarchy to know if we are in reset) so I prefer not
> >> to introduce it here.
> >> Moreover I think it has drawbacks if we want to handle complex reset
> >> use cases with more reset type.
> >> Anyway, as long as we don't migrate the reset-related state, there is
> >> no problem to switch between approaches.
> >> ---
> > 
> > So, I certainly prefer the more general "reset type" approach taken in
> > this version.  That said, I find it pretty hard to imagine what types
> > of reset other than cold will exist that have well enough defined
> > semantics to be meaningfully used from an external subsystem.
> 
> Maybe I should completely remove the type then ?

That makes sense to me.  I don't know if other possible users of the
mechanism have different opinions though.

> > 
> >>  Makefile.objs   |   1 +
> >>  hw/core/Makefile.objs   |   1 +
> >>  hw/core/resettable.c| 186 
> >>  hw/core/trace-events|  36 
> >>  include/hw/resettable.h | 159 ++
> >>  5 files changed, 383 insertions(+)
> >>  create mode 100644 hw/core/resettable.c
> >>  create mode 100644 hw/core/trace-events
> >>  create mode 100644 include/hw/resettable.h
> >>
> >> diff --git a/Makefile.objs b/Makefile.objs
> >> index 6a143dcd57..a723a47e14 100644
> >> --- a/Makefile.objs
> >> +++ b/Makefile.objs
> >> @@ -191,6 +191,7 @@ trace-events-subdirs += migration
> >>  trace-events-subdirs += net
> >>  trace-events-subdirs += ui
> >>  endif
> >> +trace-events-subdirs += hw/core
> >>  trace-events-subdirs += hw/display
> >>  trace-events-subdirs += qapi
> >>  trace-events-subdirs += qom
> >> diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
> >> index b49f880a0c..69b408ad1c 100644
> >> --- a/hw/core/Makefile.objs
> >> +++ b/hw/core/Makefile.objs
> >> @@ -1,6 +1,7 @@
> >>  # core qdev-related obj files, also used by *-user:
> >>  common-obj-y += qdev.o qdev-properties.o
> >>  common-obj-y += bus.o reset.o
> >> +common-obj-y += resettable.o
> >>  common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
> >>  common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
> >>  # irq.o needed for qdev GPIO handling:
> >> diff --git a/hw/core/resettable.c b/hw/core/resettable.c
> >> new file mode 100644
> >> index 00..b534c2c7a4
> >> --- /dev/null
> >> +++ b/hw/core/resettable.c
> >> @@ -0,0 +1,186 @@
> >> +/*
> >> + * Resettable interface.
> >> + *
> >> + * Copyright (c) 2019 GreenSocs SAS
> >> + *
> >> + * Authors:
> >> + *   Damien Hedde
> >> + *
> >> + * This work is licensed under the terms of the GNU GPL, version 2 or 
> >> later.
> >> + * See the COPYING file in the top-level directory.
> >> + */
> >> +
> >> +#include "qemu/osdep.h"
> >> +#include "qemu/module.h"
> >> +#include "hw/resettable.h"
> >> +#include "trace.h"
> >> +
> >> +#define RESETTABLE_GET_CLASS(obj) \
> >> +OBJECT_GET_CLASS(ResettableClass, (obj), TYPE_RESETTABLE_INTERFACE)
> >> +
> >> +static void resettable_foreach_child(ResettableClass *

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Thomas Huth]

On 18/09/2019 10.55, Kevin Wolf wrote:
> Running iotests is not required to build QEMU, so we can have stricter
> version requirements for Python here and can make use of new features
> and drop compatibility code earlier.
> 
> This makes qemu-iotests skip all Python tests if a Python version before
> 3.5 is used for the build.
> 
> Suggested-by: Eduardo Habkost 
> Signed-off-by: Kevin Wolf 
> ---
>  tests/qemu-iotests/check | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 875399d79f..a68f414d6c 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -633,6 +633,13 @@ then
>  export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
>  fi
>  
> +# Note that if the Python conditional here evaluates True we will exit
> +# with status 1 which is a shell 'false' value.
> +python_usable=false
> +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; then
> +python_usable=true
> +fi
> +
>  default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
>  default_alias_machine=$($QEMU_PROG -machine help | \
> sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
> @@ -809,7 +816,12 @@ do
>  start=$(_wallclock)
>  
>  if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env 
> python" ]; then
> -run_command="$PYTHON $seq"
> +if $python_usable; then
> +run_command="$PYTHON $seq"
> +else
> +run_command="false"
> +echo "Unsupported Python version" > $seq.notrun
> +fi
>  else
>  run_command="./$seq"
>  fi
> 

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v2 1/2] trace: Remove trailing newline in events

[Stefan Hajnoczi]

On Mon, Sep 16, 2019 at 12:41:44PM -0400, John Snow wrote:
> On 9/16/19 12:40 PM, Philippe Mathieu-Daudé wrote:
> > On 9/16/19 6:36 PM, Eric Blake wrote:
> >> On 9/16/19 4:51 AM, Philippe Mathieu-Daudé wrote:
> >>> While the tracing frawework does not forbid trailing newline in
> >>
> >> framework
> >>
> >>> events format string, using them lead to confuse output.
> >>> It is the responsibility of the backend to properly end an event
> >>> line.
> >>
> >> Why just trailing newline? Should we not forbid ALL use of newline in a
> >> trace message?
> > 
> > I thought about it and forgot to add a comment when respining.
> > Yes, I think this is the right thing to enforce.
> > However it requires more cleanup, affecting more subsystems, so I'd
> > rather keep it for a follow-up series.
> 
> What's the problem with using multi-line trace statements?

I agree with avoiding trailing newlines in format strings in the
./trace-events files for consistency.

Although non-trailing newlines in format strings are unusual, they could
make trace logs easier to read in complex cases.  I don't see a reason
to forbid them since we support trace backends that emit a binary log
with a parsing API - there's no need to write scripts that parse the
text output.

> (I think I intentionally use these for AHCI in a few places.)

hw/ide/ahci.c format strings do not contain newlines, so they are
technically in compliance.

ahci_pretty_buffer_fis() is used to generate a hex dump with newlines
but it's a %s format string argument to trace_handle_reg_h2d_fis_dump()
and trace_handle_cmd_fis_dump().

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices

[Kevin Wolf]

Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben:
> From: Pavel Dovgalyuk 
> 
> This patch updates the description of the command lines for using
> record/replay with attached block devices.
> 
> Signed-off-by: Pavel Dovgalyuk 
> ---
>  docs/replay.txt |   12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/replay.txt b/docs/replay.txt
> index ee6aee9861..ce97c3f72f 100644
> --- a/docs/replay.txt
> +++ b/docs/replay.txt
> @@ -27,7 +27,7 @@ Usage of the record/replay:
>   * First, record the execution with the following command line:
>  qemu-system-i386 \
>   -icount shift=7,rr=record,rrfile=replay.bin \
> - -drive file=disk.qcow2,if=none,id=img-direct \
> + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
>   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
>   -device ide-hd,drive=img-blkreplay \
>   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> @@ -35,7 +35,7 @@ Usage of the record/replay:
>   * After recording, you can replay it by using another command line:
>  qemu-system-i386 \
>   -icount shift=7,rr=replay,rrfile=replay.bin \
> - -drive file=disk.qcow2,if=none,id=img-direct \
> + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
>   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
>   -device ide-hd,drive=img-blkreplay \
>   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> @@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls of
>  bdrv coroutine functions at the top of block drivers stack.
>  To record and replay block operations the drive must be configured
>  as following:
> - -drive file=disk.qcow2,if=none,id=img-direct
> + -drive file=disk.qcow2,if=none,snapshot,id=img-direct
>   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
>   -device ide-hd,drive=img-blkreplay
>  
> @@ -252,6 +252,12 @@ This snapshot is created at start of recording and 
> restored at start
>  of replaying. It also can be loaded while replaying to roll back
>  the execution.
>  
> +'snapshot' flag of the disk image must be removed to save the snapshots
> +in the overlay (or original image) instead of using the temporary overlay.
> + -drive file=disk.ovl,if=none,id=img-direct
> + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> + -device ide-hd,drive=img-blkreplay

Wait, didn't patch 2 just make -snapshot unconditionally incompatible
with replay? So isn't saving the snapshot in the original image the only
supported mode now?

Kevin

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Max Reitz]

On 18.09.19 10:55, Kevin Wolf wrote:
> Running iotests is not required to build QEMU, so we can have stricter
> version requirements for Python here and can make use of new features
> and drop compatibility code earlier.
> 
> This makes qemu-iotests skip all Python tests if a Python version before
> 3.5 is used for the build.
> 
> Suggested-by: Eduardo Habkost 
> Signed-off-by: Kevin Wolf 
> ---
>  tests/qemu-iotests/check | 14 +-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 875399d79f..a68f414d6c 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -633,6 +633,13 @@ then
>  export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
>  fi
>  
> +# Note that if the Python conditional here evaluates True we will exit
> +# with status 1 which is a shell 'false' value.

I’d expect everything to exit with 1 if something does not work.  Thus,
I find the short script confusing (I think you do, too, or you wouldn’t
have written this comment).  Why not make it “sys.exit(0 if
sys.version_info >= (3, 5) else 1)”?

> +python_usable=false
> +if ! $PYTHON -c 'import sys; sys.exit(sys.version_info >= (3,5))'; then
> +python_usable=true
> +fi
> +
>  default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
>  default_alias_machine=$($QEMU_PROG -machine help | \
> sed -n "/(alias of $default_machine)/ { s/ .*//p; q; }")
> @@ -809,7 +816,12 @@ do
>  start=$(_wallclock)
>  
>  if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env 
> python" ]; then
> -run_command="$PYTHON $seq"
> +if $python_usable; then
> +run_command="$PYTHON $seq"
> +else
> +run_command="false"
> +echo "Unsupported Python version" > $seq.notrun
> +fi
>  else
>  run_command="./$seq"
>  fi

But it isn’t wrong, so I suppose:

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [Qemu-block] [PATCH v2 0/2] trace: Forbid trailing newline in event format

[Stefan Hajnoczi]

On Mon, Sep 16, 2019 at 11:51:19AM +0200, Philippe Mathieu-Daudé wrote:
> Hi Stefan,
> 
> I'v been confused by trailing newline in trace reports,
> so this series aims to fix this, by cleaning current
> formats and add a check to catch new one introduced.
> 
> v2:
> - Use regex format (easier to review)
> - Added R-b
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (2):
>   trace: Remove trailing newline in events
>   trace: Forbid event format ending with newline character
> 
>  docs/devel/tracing.txt|  2 ++
>  hw/misc/trace-events  | 10 +-
>  hw/scsi/trace-events  |  2 +-
>  hw/sd/trace-events|  2 +-
>  nbd/trace-events  |  4 ++--
>  net/trace-events  |  6 +++---
>  scripts/tracetool/__init__.py |  3 +++
>  7 files changed, 17 insertions(+), 12 deletions(-)

We can continue the broader discussion about whether trace events may
contain newlines in string arguments (John's AHCI case) or in
non-trailing position in the format string.  These patches look fine
though and are ready to go.

Thanks, applied (with Eric's typo fix) to my tracing tree:
https://github.com/stefanha/qemu/commits/tracing

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben:
> > From: Pavel Dovgalyuk 
> >
> > This patch updates the description of the command lines for using
> > record/replay with attached block devices.
> >
> > Signed-off-by: Pavel Dovgalyuk 
> > ---
> >  docs/replay.txt |   12 +---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/docs/replay.txt b/docs/replay.txt
> > index ee6aee9861..ce97c3f72f 100644
> > --- a/docs/replay.txt
> > +++ b/docs/replay.txt
> > @@ -27,7 +27,7 @@ Usage of the record/replay:
> >   * First, record the execution with the following command line:
> >  qemu-system-i386 \
> >   -icount shift=7,rr=record,rrfile=replay.bin \
> > - -drive file=disk.qcow2,if=none,id=img-direct \
> > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> >   -device ide-hd,drive=img-blkreplay \
> >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > @@ -35,7 +35,7 @@ Usage of the record/replay:
> >   * After recording, you can replay it by using another command line:
> >  qemu-system-i386 \
> >   -icount shift=7,rr=replay,rrfile=replay.bin \
> > - -drive file=disk.qcow2,if=none,id=img-direct \
> > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> >   -device ide-hd,drive=img-blkreplay \
> >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls of
> >  bdrv coroutine functions at the top of block drivers stack.
> >  To record and replay block operations the drive must be configured
> >  as following:
> > - -drive file=disk.qcow2,if=none,id=img-direct
> > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct
> >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> >   -device ide-hd,drive=img-blkreplay
> >
> > @@ -252,6 +252,12 @@ This snapshot is created at start of recording and 
> > restored at start
> >  of replaying. It also can be loaded while replaying to roll back
> >  the execution.
> >
> > +'snapshot' flag of the disk image must be removed to save the snapshots
> > +in the overlay (or original image) instead of using the temporary overlay.
> > + -drive file=disk.ovl,if=none,id=img-direct
> > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > + -device ide-hd,drive=img-blkreplay
> 
> Wait, didn't patch 2 just make -snapshot unconditionally incompatible
> with replay? So isn't saving the snapshot in the original image the only
> supported mode now?

There are two ways to run record/replay:
1. Disk with snapshot option and any image to make it unchanged
2. Disk with overlay without snapshot option to save VM snapshots on it

Pavel Dovgalyuk

Re: [Qemu-devel] [PULL 3/8] m68k: Add NeXTcube machine

[Thomas Huth]

On 16/09/2019 12.48, Peter Maydell wrote:
> On Sat, 7 Sep 2019 at 16:47, Thomas Huth  wrote:
>>
>> It is still quite incomplete (no SCSI, no floppy emulation, no network,
>> etc.), but the firmware already shows up the debug monitor prompt in the
>> framebuffer display, so at least the very basics are already working.
>>
>> This code has been taken from Bryce Lanham's GSoC 2011 NeXT branch at
>>
>>  https://github.com/blanham/qemu-NeXT/blob/next-cube/hw/next-cube.c
>>
>> and altered quite a bit to fit the latest interface and coding conventions
>> of the current QEMU.
>>
>> Tested-by: Philippe Mathieu-Daudé 
>> Message-Id: <20190831074519.32613-4-h...@tuxfamily.org>
>> Signed-off-by: Thomas Huth  
> Hi; Coverity spotted an issue in this function
> (CID 1405664):
> 
> 
>> +static void nextscr2_write(NeXTState *s, uint32_t val, int size)
>> +{
>> +static int led;
>> +static int phase;
>> +static uint8_t old_scr2;
>> +static uint8_t rtc_command;
>> +static uint8_t rtc_value;
>> +static uint8_t rtc_status = 0x90;
>> +static uint8_t rtc_return;
>> +uint8_t scr2_2;
>> +
> 
> 
>> +/* read the status 0x31 */
>> +if (rtc_command == 0x31) {
>> +scr2_2 = scr2_2 & (~SCR2_RTDATA);
>> +/* for now 0x00 */
>> +if (0x00 & (0x80 >> (phase - 8))) {
> 
> 0 & anything can never be true, so the line below here is dead code.

Right. I'm going to have a closer look at this at the weekend, to see
what's the best way to fix it or whether it should simply be removed.

>> +scr2_2 |= SCR2_RTDATA;
>> +}
>> +rtc_return = (rtc_return << 1) |
>> + ((scr2_2 & SCR2_RTDATA) ? 1 : 0);
>> +}
>> +
> 
> Incidentally, I see that this file has quite a lot of
> what seems to be essentially device emulation code in it
> (a bunch of IO MemoryRegions defined locally) -- ideally
> these could be split out into proper device objects at
> some point.

Yeah, it's all old code from 2011 ... I'll keep this in mind for future
clean-ups!

 Thanks,
  Thomas

Re: [Qemu-devel] [PATCH v3 00/29] s390x/tcg: mem_helper: Fault-safe handling

[Cornelia Huck]

On Wed, 18 Sep 2019 10:25:15 +0200
David Hildenbrand  wrote:

> On 16.09.19 15:57, David Hildenbrand wrote:
> > This series fixes a bunch of issues related to some mem helpers and makes
> > sure that they are fault-safe, meaning no system state is modified in case
> > a fault is triggered.
> > 
> > I can spot tons of other issues with other mem helpers that will have
> > to be fixed later. Also, fault-safe handling for some instructions
> > (especially TR) might be harder to implement (you don't know what will
> > actually be accessed upfront - we might need a buffer and go over
> > inputs twice). Focusing on the MOVE instructions for now.
> > 
> > 
> > 
> > Newer versions of glibc use memcpy() in memmove() for forward moves. The
> > implementation makese use of MVC. The TCG implementation of MVC is
> > currently not able to handle faults reliably when crossing pages. MVC
> > can cross with 256 bytes at most two pages.
> > 
> > In case we get a fault on the second page, we already moved data. When
> > continuing after the fault we might try to move already overwritten data,
> > which is very bad in case we have overlapping data on a forward move.
> > 
> > Triggered for now only by rpmbuild (crashes when checking the spec file)
> > and rpm (database corruptions). This fixes installing Fedora rawhide (31)
> > under TCG.
> > 
> > This was horrible to debug as it barely triggers and we fail at completely
> > different places.
> > 
> > Cc: Stefano Brivio 
> > Cc: Florian Weimer 
> > Cc: Dan Horák 
> > Cc: Cole Robinson 
> > 
> > v2 -> v3:
> > - "s390x/tcg: MVCL: Zero out unused bits of address"
> > -- Do single deposit for 24/31-bit
> > - "s390x/tcg: MVCL: Process max 4k bytes at a time"
> > -- Use max of 4k instead of 2k, limiting to single pages
> > - "s390x/tcg: MVCLU/MVCLE: Process max 4k bytes at a time"
> > -- Limit to single pages
> > - "s390x/tcg: MVCOS: Lengths are 32 bit in 24/31-bit mode"
> > -- Added
> > - "s390x/tcg: MVCS/MVCP: Properly wrap the length"
> > -- Properly use 32 instead of 31 bit.
> > - "s390x/tcg: MVST: Fix storing back the addresses to registers"
> > -- Read R0 implicitly
> > - "s390x/tcg: Fault-safe memset"
> > -- Speed up TLB_NOTDIRTY handling
> > -- Move single-page access to helper function
> > -- Pass access structure to access_memset()
> > -- Replace access_prepare() by previous access_prepare_idx()
> > - "s390x/tcg: Fault-safe memmove"
> > -- Pass access structure to access_memmove()
> > -- Speed up TLB_NOTDIRTY handling when accessing single bytes
> > - The other fault-safe handling patches were adapted to work with the
> >   changed access functions. mmu_idx is now always passed to
> >   access_prepare() from the helpers.
> > 
> > v1 -> v2:
> > - Include many fixes
> > - Fix more instructions
> > - Use the new probe_access() function
> > - Include "tests/tcg: target/s390x: Test MVO"
> > 
> > David Hildenbrand (29):
> >   s390x/tcg: Reset exception_index to -1 instead of 0
> >   s390x/tcg: MVCL: Zero out unused bits of address
> >   s390x/tcg: MVCL: Detect destructive overlaps
> >   s390x/tcg: MVCL: Process max 4k bytes at a time
> >   s390x/tcg: MVC: Increment the length once
> >   s390x/tcg: MVC: Use is_destructive_overlap()
> >   s390x/tcg: MVPG: Check for specification exceptions
> >   s390x/tcg: MVPG: Properly wrap the addresses
> >   s390x/tcg: MVCLU/MVCLE: Process max 4k bytes at a time
> >   s390x/tcg: MVCS/MVCP: Check for special operation exceptions
> >   s390x/tcg: MVCOS: Lengths are 32 bit in 24/31-bit mode
> >   s390x/tcg: MVCS/MVCP: Properly wrap the length
> >   s390x/tcg: MVST: Check for specification exceptions
> >   s390x/tcg: MVST: Fix storing back the addresses to registers
> >   s390x/tcg: Always use MMU_USER_IDX for CONFIG_USER_ONLY
> >   s390x/tcg: Fault-safe memset
> >   s390x/tcg: Fault-safe memmove
> >   s390x/tcg: MVCS/MVCP: Use access_memmove()
> >   s390x/tcg: MVC: Fault-safe handling on destructive overlaps
> >   s390x/tcg: MVCLU: Fault-safe handling
> >   s390x/tcg: OC: Fault-safe handling
> >   s390x/tcg: XC: Fault-safe handling
> >   s390x/tcg: NC: Fault-safe handling
> >   s390x/tcg: MVCIN: Fault-safe handling
> >   s390x/tcg: MVN: Fault-safe handling
> >   s390x/tcg: MVZ: Fault-safe handling
> >   s390x/tcg: MVST: Fault-safe handling
> >   s390x/tcg: MVO: Fault-safe handling
> >   tests/tcg: target/s390x: Test MVO
> > 
> >  target/s390x/cpu.h  |   4 +
> >  target/s390x/helper.h   |   2 +-
> >  target/s390x/insn-data.def  |   2 +-
> >  target/s390x/mem_helper.c   | 743 ++--
> >  target/s390x/translate.c|  12 +-
> >  tests/tcg/s390x/Makefile.target |   1 +
> >  tests/tcg/s390x/mvo.c   |  25 ++
> >  7 files changed, 564 insertions(+), 225 deletions(-)
> >  create mode 100644 tests/tcg/s390x/mvo.c
> >   
> 
> As long as there are no further comments, this series is ready to go
> (only one patch description needs a fixup).

I don't have any :)

> 
> Conny, how do you pre

Re: [Qemu-devel] [PATCH v3 00/29] s390x/tcg: mem_helper: Fault-safe handling

[David Hildenbrand]

On 18.09.19 11:26, Cornelia Huck wrote:
> On Wed, 18 Sep 2019 10:25:15 +0200
> David Hildenbrand  wrote:
> 
>> On 16.09.19 15:57, David Hildenbrand wrote:
>>> This series fixes a bunch of issues related to some mem helpers and makes
>>> sure that they are fault-safe, meaning no system state is modified in case
>>> a fault is triggered.
>>>
>>> I can spot tons of other issues with other mem helpers that will have
>>> to be fixed later. Also, fault-safe handling for some instructions
>>> (especially TR) might be harder to implement (you don't know what will
>>> actually be accessed upfront - we might need a buffer and go over
>>> inputs twice). Focusing on the MOVE instructions for now.
>>>
>>> 
>>>
>>> Newer versions of glibc use memcpy() in memmove() for forward moves. The
>>> implementation makese use of MVC. The TCG implementation of MVC is
>>> currently not able to handle faults reliably when crossing pages. MVC
>>> can cross with 256 bytes at most two pages.
>>>
>>> In case we get a fault on the second page, we already moved data. When
>>> continuing after the fault we might try to move already overwritten data,
>>> which is very bad in case we have overlapping data on a forward move.
>>>
>>> Triggered for now only by rpmbuild (crashes when checking the spec file)
>>> and rpm (database corruptions). This fixes installing Fedora rawhide (31)
>>> under TCG.
>>>
>>> This was horrible to debug as it barely triggers and we fail at completely
>>> different places.
>>>
>>> Cc: Stefano Brivio 
>>> Cc: Florian Weimer 
>>> Cc: Dan Horák 
>>> Cc: Cole Robinson 
>>>
>>> v2 -> v3:
>>> - "s390x/tcg: MVCL: Zero out unused bits of address"
>>> -- Do single deposit for 24/31-bit
>>> - "s390x/tcg: MVCL: Process max 4k bytes at a time"
>>> -- Use max of 4k instead of 2k, limiting to single pages
>>> - "s390x/tcg: MVCLU/MVCLE: Process max 4k bytes at a time"
>>> -- Limit to single pages
>>> - "s390x/tcg: MVCOS: Lengths are 32 bit in 24/31-bit mode"
>>> -- Added
>>> - "s390x/tcg: MVCS/MVCP: Properly wrap the length"
>>> -- Properly use 32 instead of 31 bit.
>>> - "s390x/tcg: MVST: Fix storing back the addresses to registers"
>>> -- Read R0 implicitly
>>> - "s390x/tcg: Fault-safe memset"
>>> -- Speed up TLB_NOTDIRTY handling
>>> -- Move single-page access to helper function
>>> -- Pass access structure to access_memset()
>>> -- Replace access_prepare() by previous access_prepare_idx()
>>> - "s390x/tcg: Fault-safe memmove"
>>> -- Pass access structure to access_memmove()
>>> -- Speed up TLB_NOTDIRTY handling when accessing single bytes
>>> - The other fault-safe handling patches were adapted to work with the
>>>   changed access functions. mmu_idx is now always passed to
>>>   access_prepare() from the helpers.
>>>
>>> v1 -> v2:
>>> - Include many fixes
>>> - Fix more instructions
>>> - Use the new probe_access() function
>>> - Include "tests/tcg: target/s390x: Test MVO"
>>>
>>> David Hildenbrand (29):
>>>   s390x/tcg: Reset exception_index to -1 instead of 0
>>>   s390x/tcg: MVCL: Zero out unused bits of address
>>>   s390x/tcg: MVCL: Detect destructive overlaps
>>>   s390x/tcg: MVCL: Process max 4k bytes at a time
>>>   s390x/tcg: MVC: Increment the length once
>>>   s390x/tcg: MVC: Use is_destructive_overlap()
>>>   s390x/tcg: MVPG: Check for specification exceptions
>>>   s390x/tcg: MVPG: Properly wrap the addresses
>>>   s390x/tcg: MVCLU/MVCLE: Process max 4k bytes at a time
>>>   s390x/tcg: MVCS/MVCP: Check for special operation exceptions
>>>   s390x/tcg: MVCOS: Lengths are 32 bit in 24/31-bit mode
>>>   s390x/tcg: MVCS/MVCP: Properly wrap the length
>>>   s390x/tcg: MVST: Check for specification exceptions
>>>   s390x/tcg: MVST: Fix storing back the addresses to registers
>>>   s390x/tcg: Always use MMU_USER_IDX for CONFIG_USER_ONLY
>>>   s390x/tcg: Fault-safe memset
>>>   s390x/tcg: Fault-safe memmove
>>>   s390x/tcg: MVCS/MVCP: Use access_memmove()
>>>   s390x/tcg: MVC: Fault-safe handling on destructive overlaps
>>>   s390x/tcg: MVCLU: Fault-safe handling
>>>   s390x/tcg: OC: Fault-safe handling
>>>   s390x/tcg: XC: Fault-safe handling
>>>   s390x/tcg: NC: Fault-safe handling
>>>   s390x/tcg: MVCIN: Fault-safe handling
>>>   s390x/tcg: MVN: Fault-safe handling
>>>   s390x/tcg: MVZ: Fault-safe handling
>>>   s390x/tcg: MVST: Fault-safe handling
>>>   s390x/tcg: MVO: Fault-safe handling
>>>   tests/tcg: target/s390x: Test MVO
>>>
>>>  target/s390x/cpu.h  |   4 +
>>>  target/s390x/helper.h   |   2 +-
>>>  target/s390x/insn-data.def  |   2 +-
>>>  target/s390x/mem_helper.c   | 743 ++--
>>>  target/s390x/translate.c|  12 +-
>>>  tests/tcg/s390x/Makefile.target |   1 +
>>>  tests/tcg/s390x/mvo.c   |  25 ++
>>>  7 files changed, 564 insertions(+), 225 deletions(-)
>>>  create mode 100644 tests/tcg/s390x/mvo.c
>>>   
>>
>> As long as there are no further comments, this series is ready to go
>> (only one patch description needs a fixup).
> 
> I don

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Kevin Wolf]

Am 18.09.2019 um 11:20 hat Max Reitz geschrieben:
> On 18.09.19 10:55, Kevin Wolf wrote:
> > Running iotests is not required to build QEMU, so we can have stricter
> > version requirements for Python here and can make use of new features
> > and drop compatibility code earlier.
> > 
> > This makes qemu-iotests skip all Python tests if a Python version before
> > 3.5 is used for the build.
> > 
> > Suggested-by: Eduardo Habkost 
> > Signed-off-by: Kevin Wolf 
> > ---
> >  tests/qemu-iotests/check | 14 +-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> > index 875399d79f..a68f414d6c 100755
> > --- a/tests/qemu-iotests/check
> > +++ b/tests/qemu-iotests/check
> > @@ -633,6 +633,13 @@ then
> >  export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
> >  fi
> >  
> > +# Note that if the Python conditional here evaluates True we will exit
> > +# with status 1 which is a shell 'false' value.
> 
> I’d expect everything to exit with 1 if something does not work.  Thus,
> I find the short script confusing (I think you do, too, or you wouldn’t
> have written this comment).  Why not make it “sys.exit(0 if
> sys.version_info >= (3, 5) else 1)”?

I just copied it from configure, actually. :-)

But we can use your way, too. I don't really mind.

Kevin


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices

[Kevin Wolf]

Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben:
> > From: Kevin Wolf [mailto:kw...@redhat.com]
> > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben:
> > > From: Pavel Dovgalyuk 
> > >
> > > This patch updates the description of the command lines for using
> > > record/replay with attached block devices.
> > >
> > > Signed-off-by: Pavel Dovgalyuk 
> > > ---
> > >  docs/replay.txt |   12 +---
> > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/docs/replay.txt b/docs/replay.txt
> > > index ee6aee9861..ce97c3f72f 100644
> > > --- a/docs/replay.txt
> > > +++ b/docs/replay.txt
> > > @@ -27,7 +27,7 @@ Usage of the record/replay:
> > >   * First, record the execution with the following command line:
> > >  qemu-system-i386 \
> > >   -icount shift=7,rr=record,rrfile=replay.bin \
> > > - -drive file=disk.qcow2,if=none,id=img-direct \
> > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> > >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> > >   -device ide-hd,drive=img-blkreplay \
> > >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > > @@ -35,7 +35,7 @@ Usage of the record/replay:
> > >   * After recording, you can replay it by using another command line:
> > >  qemu-system-i386 \
> > >   -icount shift=7,rr=replay,rrfile=replay.bin \
> > > - -drive file=disk.qcow2,if=none,id=img-direct \
> > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> > >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> > >   -device ide-hd,drive=img-blkreplay \
> > >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls of
> > >  bdrv coroutine functions at the top of block drivers stack.
> > >  To record and replay block operations the drive must be configured
> > >  as following:
> > > - -drive file=disk.qcow2,if=none,id=img-direct
> > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct
> > >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > >   -device ide-hd,drive=img-blkreplay
> > >
> > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording and 
> > > restored at start
> > >  of replaying. It also can be loaded while replaying to roll back
> > >  the execution.
> > >
> > > +'snapshot' flag of the disk image must be removed to save the snapshots
> > > +in the overlay (or original image) instead of using the temporary 
> > > overlay.
> > > + -drive file=disk.ovl,if=none,id=img-direct
> > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > > + -device ide-hd,drive=img-blkreplay
> > 
> > Wait, didn't patch 2 just make -snapshot unconditionally incompatible
> > with replay? So isn't saving the snapshot in the original image the only
> > supported mode now?
> 
> There are two ways to run record/replay:
> 1. Disk with snapshot option and any image to make it unchanged
> 2. Disk with overlay without snapshot option to save VM snapshots on it

Yes, I think I understand the two options that you intend to make
available, but when -snapshot sets a replay blocker, how can 1. still
work? Is there some code that ignores replay blockers?

Kevin

Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben:
> > > > From: Pavel Dovgalyuk 
> > > >
> > > > This patch updates the description of the command lines for using
> > > > record/replay with attached block devices.
> > > >
> > > > Signed-off-by: Pavel Dovgalyuk 
> > > > ---
> > > >  docs/replay.txt |   12 +---
> > > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/docs/replay.txt b/docs/replay.txt
> > > > index ee6aee9861..ce97c3f72f 100644
> > > > --- a/docs/replay.txt
> > > > +++ b/docs/replay.txt
> > > > @@ -27,7 +27,7 @@ Usage of the record/replay:
> > > >   * First, record the execution with the following command line:
> > > >  qemu-system-i386 \
> > > >   -icount shift=7,rr=record,rrfile=replay.bin \
> > > > - -drive file=disk.qcow2,if=none,id=img-direct \
> > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> > > >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay 
> > > > \
> > > >   -device ide-hd,drive=img-blkreplay \
> > > >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > > > @@ -35,7 +35,7 @@ Usage of the record/replay:
> > > >   * After recording, you can replay it by using another command line:
> > > >  qemu-system-i386 \
> > > >   -icount shift=7,rr=replay,rrfile=replay.bin \
> > > > - -drive file=disk.qcow2,if=none,id=img-direct \
> > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> > > >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay 
> > > > \
> > > >   -device ide-hd,drive=img-blkreplay \
> > > >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts calls 
> > > > of
> > > >  bdrv coroutine functions at the top of block drivers stack.
> > > >  To record and replay block operations the drive must be configured
> > > >  as following:
> > > > - -drive file=disk.qcow2,if=none,id=img-direct
> > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct
> > > >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > > >   -device ide-hd,drive=img-blkreplay
> > > >
> > > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording and 
> > > > restored at
> start
> > > >  of replaying. It also can be loaded while replaying to roll back
> > > >  the execution.
> > > >
> > > > +'snapshot' flag of the disk image must be removed to save the snapshots
> > > > +in the overlay (or original image) instead of using the temporary 
> > > > overlay.
> > > > + -drive file=disk.ovl,if=none,id=img-direct
> > > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > > > + -device ide-hd,drive=img-blkreplay
> > >
> > > Wait, didn't patch 2 just make -snapshot unconditionally incompatible
> > > with replay? So isn't saving the snapshot in the original image the only
> > > supported mode now?
> >
> > There are two ways to run record/replay:
> > 1. Disk with snapshot option and any image to make it unchanged
> > 2. Disk with overlay without snapshot option to save VM snapshots on it
> 
> Yes, I think I understand the two options that you intend to make
> available, but when -snapshot sets a replay blocker, how can 1. still
> work? Is there some code that ignores replay blockers?

I checked the text and don't see anything about global "-snapshot" option.
All references are about drive snapshot flag.
Can you point me where did I missed that?

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH] libvhost-user: handle NOFD flag in call/kick/err better

[Stefan Hajnoczi]

On Tue, Sep 17, 2019 at 02:25:59PM +0200, Johannes Berg wrote:
> diff --git a/contrib/libvhost-user/libvhost-user.c 
> b/contrib/libvhost-user/libvhost-user.c
> index f1677da21201..17b7833d1f6b 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -920,6 +920,7 @@ static bool
>  vu_check_queue_msg_file(VuDev *dev, VhostUserMsg *vmsg)
>  {
>  int index = vmsg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
> +bool nofd = vmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK;
>  
>  if (index >= dev->max_queues) {
>  vmsg_close_fds(vmsg);
> @@ -927,8 +928,12 @@ vu_check_queue_msg_file(VuDev *dev, VhostUserMsg *vmsg)
>  return false;
>  }
>  
> -if (vmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK ||
> -vmsg->fd_num != 1) {
> +if (nofd) {
> +vmsg_close_fds(vmsg);
> +return true;
> +}

With the following change to vmsg_close_fds():

  for (i = 0; i < vmsg->fd_num; i++) {
  close(vmsg->fds[i]);
  }
+ for (i = 0; i < sizeof(vmsg->fd_num) / sizeof(vmsg->fd_num[0]); i++) {
+ vmsg->fds[i] = -1;
+ }
+ vmsg->fd_num = 0;

...the message handler functions below can use vmsg->fds[0] (-1) without
worrying about NOFD.  This makes the code simpler.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices

[Kevin Wolf]

Am 18.09.2019 um 11:37 hat Pavel Dovgalyuk geschrieben:
> > From: Kevin Wolf [mailto:kw...@redhat.com]
> > Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben:
> > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben:
> > > > > From: Pavel Dovgalyuk 
> > > > >
> > > > > This patch updates the description of the command lines for using
> > > > > record/replay with attached block devices.
> > > > >
> > > > > Signed-off-by: Pavel Dovgalyuk 
> > > > > ---
> > > > >  docs/replay.txt |   12 +---
> > > > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/docs/replay.txt b/docs/replay.txt
> > > > > index ee6aee9861..ce97c3f72f 100644
> > > > > --- a/docs/replay.txt
> > > > > +++ b/docs/replay.txt
> > > > > @@ -27,7 +27,7 @@ Usage of the record/replay:
> > > > >   * First, record the execution with the following command line:
> > > > >  qemu-system-i386 \
> > > > >   -icount shift=7,rr=record,rrfile=replay.bin \
> > > > > - -drive file=disk.qcow2,if=none,id=img-direct \
> > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> > > > >   -drive 
> > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> > > > >   -device ide-hd,drive=img-blkreplay \
> > > > >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > > > > @@ -35,7 +35,7 @@ Usage of the record/replay:
> > > > >   * After recording, you can replay it by using another command line:
> > > > >  qemu-system-i386 \
> > > > >   -icount shift=7,rr=replay,rrfile=replay.bin \
> > > > > - -drive file=disk.qcow2,if=none,id=img-direct \
> > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> > > > >   -drive 
> > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> > > > >   -device ide-hd,drive=img-blkreplay \
> > > > >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > > > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts 
> > > > > calls of
> > > > >  bdrv coroutine functions at the top of block drivers stack.
> > > > >  To record and replay block operations the drive must be configured
> > > > >  as following:
> > > > > - -drive file=disk.qcow2,if=none,id=img-direct
> > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct
> > > > >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > > > >   -device ide-hd,drive=img-blkreplay
> > > > >
> > > > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording 
> > > > > and restored at
> > start
> > > > >  of replaying. It also can be loaded while replaying to roll back
> > > > >  the execution.
> > > > >
> > > > > +'snapshot' flag of the disk image must be removed to save the 
> > > > > snapshots
> > > > > +in the overlay (or original image) instead of using the temporary 
> > > > > overlay.
> > > > > + -drive file=disk.ovl,if=none,id=img-direct
> > > > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > > > > + -device ide-hd,drive=img-blkreplay
> > > >
> > > > Wait, didn't patch 2 just make -snapshot unconditionally incompatible
> > > > with replay? So isn't saving the snapshot in the original image the only
> > > > supported mode now?
> > >
> > > There are two ways to run record/replay:
> > > 1. Disk with snapshot option and any image to make it unchanged
> > > 2. Disk with overlay without snapshot option to save VM snapshots on it
> > 
> > Yes, I think I understand the two options that you intend to make
> > available, but when -snapshot sets a replay blocker, how can 1. still
> > work? Is there some code that ignores replay blockers?
> 
> I checked the text and don't see anything about global "-snapshot" option.
> All references are about drive snapshot flag.
> Can you point me where did I missed that?

Oh, sorry, you're right and I misread.

However, global -snapshot is just a convenient shortcut for specifying
snapshot=on for all -drive arguments. So if -snapshot is incompatible
with replay, shouldn't manually marking all drives as snapshot=on be
incompatible as well?

Maybe you're really interested in some specific drive not having
snapshot=on? But then it might be better to check that specific drive
instad of forbidding just the shortcut for setting it.

Kevin

Re: [Qemu-devel] [PATCH 1/2] audio: fix buffer-length typo in documentation

[Stefan Hajnoczi]

On Tue, Sep 17, 2019 at 09:29:34PM +0200, Zoltán Kővágó wrote:
> On 2019-09-11 16:58, Stefan Hajnoczi wrote:
> > Fixes: f0b3d811529 ("audio: -audiodev command line option: documentation")
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >   qemu-options.hx | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index bbfd936d29..a4f9f74f52 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -439,7 +439,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
> >   "in|out.format= sample format to use with fixed 
> > settings\n"
> >   "valid values: s8, s16, s32, u8, u16, u32\n"
> >   "in|out.voices= number of voices to use\n"
> > -"in|out.buffer-len= length of buffer in microseconds\n"
> > +"in|out.buffer-length= length of buffer in 
> > microseconds\n"
> >   "-audiodev none,id=id,[,prop[=value][,...]]\n"
> >   "dummy driver that discards all output\n"
> >   #ifdef CONFIG_AUDIO_ALSA
> > @@ -524,7 +524,7 @@ Valid values are: @code{s8}, @code{s16}, @code{s32}, 
> > @code{u8},
> >   @item in|out.voices=@var{voices}
> >   Specify the number of @var{voices} to use.  Default is 1.
> > -@item in|out.buffer=@var{usecs}
> > +@item in|out.buffer-length=@var{usecs}
> >   Sets the size of the buffer in microseconds.
> >   @end table
> > 
> 
> Double checking it's indeed "buffer-length" in qapi.  Also I spot a
> different bug: the alsa documentation qemu-options.hx has "period-len" but
> according to qapi it should be "period-length".  Care to fix it or should I
> submit a different patch?

Thanks.  I will send another revision.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 29/29] tests/tcg: target/s390x: Test MVO

[Alex Bennée]

David Hildenbrand  writes:

> Let's add the simple test based on the example from the PoP.
>
> Signed-off-by: David Hildenbrand 
> ---
>  tests/tcg/s390x/Makefile.target |  1 +
>  tests/tcg/s390x/mvo.c   | 25 +
>  2 files changed, 26 insertions(+)
>  create mode 100644 tests/tcg/s390x/mvo.c
>
> diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
> index 151dc075aa..6a3bfa8b29 100644
> --- a/tests/tcg/s390x/Makefile.target
> +++ b/tests/tcg/s390x/Makefile.target
> @@ -6,3 +6,4 @@ TESTS+=ipm
>  TESTS+=exrl-trt
>  TESTS+=exrl-trtr
>  TESTS+=pack
> +TESTS+=mvo
> diff --git a/tests/tcg/s390x/mvo.c b/tests/tcg/s390x/mvo.c
> new file mode 100644
> index 00..5546fe2a97
> --- /dev/null
> +++ b/tests/tcg/s390x/mvo.c
> @@ -0,0 +1,25 @@
> +#include 
> +#include 
> +
> +int main(void)
> +{
> +uint8_t dest[6] = {0xff, 0x77, 0x88, 0x99, 0x0c, 0xff};
> +uint8_t src[5] = {0xee, 0x12, 0x34, 0x56, 0xee};
> +uint8_t expected[6] = {0xff, 0x01, 0x23, 0x45, 0x6c, 0xff};
> +int i;
> +
> +asm volatile (
> +"mvo 0(4,%[dest]),0(3,%[src])\n"
> +:
> +: [dest] "d" (dest + 1),
> +  [src] "d" (src + 1)
> +: "memory");
> +
> +for (i = 0; i < sizeof(expected); i++) {
> +if (dest[i] != expected[i]) {
> +fprintf(stderr, "bad data\n");
> +return 1;
> +}
> +}
> +return 0;
> +}

Reviewed-by: Alex Bennée 

but...

can this test be expanded to check the page cross cases that caused you
so much trouble to track down?

--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/2] audio: add -audiodev pa, in|out.latency= to documentation

[Stefan Hajnoczi]

On Tue, Sep 17, 2019 at 09:47:11PM +0200, Zoltán Kővágó wrote:
> On 2019-09-11 16:58, Stefan Hajnoczi wrote:
> > The "latency" parameter wasn't covered by the documentation.
> > 
> > Signed-off-by: Stefan Hajnoczi 
> Reviewed-by: Zoltán Kővágó 
> > ---
> > How is this parameter related to buffer-length?
> 
> Pulseaudio being a client-server architecture is a bit different than the
> other backends, plus it also has to mix multiple streams. buffer-length
> corresponds to the buffer inside qemu, while latency corresponds to
> pulseaudio.  For playback, the latency should be "maximum latency that the
> application can deal with", if a different client request a lower latency,
> our latency will decrease too.  It's up to the server to figure out an
> optimal buffer size on the server side of the things.
> For recording it's the size of the buffer we will read at a time from
> pulseaudio.

Thanks for explaining.  I will expand the help text in v2.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] libvhost-user: handle NOFD flag in call/kick/err better

[Johannes Berg]

On Wed, 2019-09-18 at 10:39 +0100, Stefan Hajnoczi wrote:
> 
> >  vu_check_queue_msg_file(VuDev *dev, VhostUserMsg *vmsg)
> >  {
> >  int index = vmsg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
> > +bool nofd = vmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK;
> >  
> >  if (index >= dev->max_queues) {
> >  vmsg_close_fds(vmsg);
> > @@ -927,8 +928,12 @@ vu_check_queue_msg_file(VuDev *dev, VhostUserMsg *vmsg)
> >  return false;
> >  }
> >  
> > -if (vmsg->payload.u64 & VHOST_USER_VRING_NOFD_MASK ||
> > -vmsg->fd_num != 1) {
> > +if (nofd) {
> > +vmsg_close_fds(vmsg);
> > +return true;
> > +}

So in this particular code you quoted, I actually just aligned to have
the same "bool nofd" variable - and I made it return "true" when no FD
was given.

It couldn't make use of what you proposed:

> With the following change to vmsg_close_fds():
> 
>   for (i = 0; i < vmsg->fd_num; i++) {
>   close(vmsg->fds[i]);
>   }
> + for (i = 0; i < sizeof(vmsg->fd_num) / sizeof(vmsg->fd_num[0]); i++) {
> + vmsg->fds[i] = -1;
> + }
> + vmsg->fd_num = 0;
> 
> ...the message handler functions below can use vmsg->fds[0] (-1) without
> worrying about NOFD.  This makes the code simpler.

because fd_num != 1 leads to the original code returning false, which
leads to the ring not getting started in vu_set_vring_kick_exec(). So we
need the special code here, can be argued if I should pull out the test
into the "bool nofd" variable or not ... *shrug*

The changes in vu_set_vring_kick_exec() and vu_set_vring_err_exec()
would indeed then not be necessary, but in vu_set_vring_call_exec() we
should still avoid the eventfd_write() if it's going to get -1.


So, yeah - could be a bit simpler there. I'd say being explicit here is
easier to understand and thus nicer, but your (or Michael's I guess?)
call.

johannes

[Qemu-devel] [PATCH 5/8] block: Evaluate @exact in protocol drivers

[Max Reitz]

We have two protocol drivers that return success when trying to shrink a
block device even though they cannot shrink it.  This behavior is now
only allowed with exact=false, so they should return an error with
exact=true.

Signed-off-by: Max Reitz 
---
 block/file-posix.c | 8 +++-
 block/iscsi.c  | 7 ++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index d8755c5fac..a85f3486d1 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2028,6 +2028,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState 
*bs, int64_t offset,
 }
 
 if (S_ISREG(st.st_mode)) {
+/* Always resizes to the exact @offset */
 return raw_regular_truncate(bs, s->fd, offset, prealloc, errp);
 }
 
@@ -2038,7 +2039,12 @@ static int coroutine_fn raw_co_truncate(BlockDriverState 
*bs, int64_t offset,
 }
 
 if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
-if (offset > raw_getlength(bs)) {
+int64_t cur_length = raw_getlength(bs);
+
+if (offset != cur_length && exact) {
+error_setg(errp, "Cannot resize device files");
+return -ENOTSUP;
+} else if (offset > cur_length) {
 error_setg(errp, "Cannot grow device files");
 return -EINVAL;
 }
diff --git a/block/iscsi.c b/block/iscsi.c
index a90426270a..cc2072ff6c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2126,6 +2126,7 @@ static int coroutine_fn 
iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
   Error **errp)
 {
 IscsiLun *iscsilun = bs->opaque;
+int64_t cur_length;
 Error *local_err = NULL;
 
 if (prealloc != PREALLOC_MODE_OFF) {
@@ -2145,7 +2146,11 @@ static int coroutine_fn 
iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
 return -EIO;
 }
 
-if (offset > iscsi_getlength(bs)) {
+cur_length = iscsi_getlength(bs);
+if (offset != cur_length && exact) {
+error_setg(errp, "Cannot resize iSCSI devices");
+return -ENOTSUP;
+} else if (offset > cur_length) {
 error_setg(errp, "Cannot grow iSCSI devices");
 return -EINVAL;
 }
-- 
2.21.0

[Qemu-devel] [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate()

[Max Reitz]

We have two drivers (iscsi and file-posix) that (in some cases) return
success from their .bdrv_co_truncate() implementation if the block
device is larger than the requested offset, but cannot be shrunk.  Some
callers do not want that behavior, so this patch adds a new parameter
that they can use to turn off that behavior.

This patch just adds the parameter and lets the block/io.c and
block/block-backend.c functions pass it around.  All other callers
always pass false and none of the implementations evaluate it, so that
this patch does not change existing behavior.  Future patches take care
of that.

Suggested-by: Maxim Levitsky 
Signed-off-by: Max Reitz 
---
 include/block/block.h  |  6 +++---
 include/block/block_int.h  | 17 -
 include/sysemu/block-backend.h |  4 ++--
 block/block-backend.c  |  6 +++---
 block/commit.c |  5 +++--
 block/crypto.c |  8 
 block/file-posix.c |  3 ++-
 block/file-win32.c |  3 ++-
 block/gluster.c|  1 +
 block/io.c | 20 +---
 block/iscsi.c  |  3 ++-
 block/mirror.c |  4 ++--
 block/nfs.c|  2 +-
 block/parallels.c  |  6 +++---
 block/qcow.c   |  4 ++--
 block/qcow2-refcount.c |  2 +-
 block/qcow2.c  | 22 --
 block/qed.c|  3 ++-
 block/raw-format.c |  5 +++--
 block/rbd.c|  1 +
 block/sheepdog.c   |  5 +++--
 block/ssh.c|  3 ++-
 block/vdi.c|  2 +-
 block/vhdx-log.c   |  4 ++--
 block/vhdx.c   |  7 ---
 block/vmdk.c   |  8 
 block/vpc.c|  2 +-
 blockdev.c |  2 +-
 qemu-img.c |  2 +-
 qemu-io-cmds.c |  2 +-
 tests/test-block-iothread.c|  8 
 31 files changed, 102 insertions(+), 68 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 37c9de7446..2f905ae4b7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -346,10 +346,10 @@ BlockDriverState 
*bdrv_find_backing_image(BlockDriverState *bs,
 const char *backing_file);
 void bdrv_refresh_filename(BlockDriverState *bs);
 
-int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
+int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
   PreallocMode prealloc, Error **errp);
-int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
-  Error **errp);
+int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
+  PreallocMode prealloc, Error **errp);
 
 int64_t bdrv_nb_sectors(BlockDriverState *bs);
 int64_t bdrv_getlength(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0422acdf1c..197cc6e7c3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -334,8 +334,23 @@ struct BlockDriver {
  * bdrv_parse_filename.
  */
 const char *protocol_name;
+
+/*
+ * Truncate @bs to @offset bytes using the given @prealloc mode
+ * when growing.  Modes other than PREALLOC_MODE_OFF should be
+ * rejected when shrinking @bs.
+ *
+ * If @exact is true, @bs must be resized to exactly @offset.
+ * Otherwise, it is sufficient for @bs (if it is a host block
+ * device and thus there is no way to resize it) to be at least
+ * @offset bytes in length.
+ *
+ * If @exact is true and this function fails but would succeed
+ * with @exact = false, it should return -ENOTSUP.
+ */
 int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp);
+ bool exact, PreallocMode prealloc,
+ Error **errp);
 
 int64_t (*bdrv_getlength)(BlockDriverState *bs);
 bool has_variable_length;
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 368d53af77..841804c3cb 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -233,8 +233,8 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, 
int64_t offset,
   int bytes, BdrvRequestFlags flags);
 int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
   int bytes);
-int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
- Error **errp);
+int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
+ PreallocMode prealloc, Error **errp);
 int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
 int blk_save_vmstate(BlockBackend *blk, const uint8_t

[Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate()

[Max Reitz]

Hi,

This series is supposed to pull out some of the problems from my
“Generic file creation fallback” series.

The blk_truncate_for_formatting() function added there was buggy, as
Maxim noted, in that it did not check whether blk_truncate() actually
resized the block node to the target offset.  One way to fix this is to
add a parameter to it that forces the block driver to do so, and that is
done by this series.

I think this is generally useful (you can see the diff stat saldo is
only +23 lines), because it allows us to drop a special check in
qemu-img resize, and it fixes a bug in qed (which has relied on this
behavior for over 8 years, but unfortunately bdrv_truncate()’s behavior
changed quite exactly 8 years ago).

However, in the process I noticed we actually don’t need
blk_truncate_for_formatting(): The underlying problem is that some
format drivers truncate their underlying file node to 0 before
formatting it to drop all data.  So they should pass exact=true, but
they cannot, because this would break creation on block devices.  Hence
blk_truncate_for_formatting().

It turns out, though, that three of the four drivers in question don’t
need to truncate their file node at all.  The remaining one is qed which
simply really should pass exact=true (it’s a bug fix).

(I do drop those blk_truncate() invocations in this series, because
otherwise I feel like it is impossible to decide whether they should get
exact=false or exact=true.  Either way is wrong.)


Max Reitz (8):
  block: Handle filter truncation like native impl.
  block/cor: Drop cor_co_truncate()
  block: Do not truncate file node when formatting
  block: Add @exact parameter to bdrv_co_truncate()
  block: Evaluate @exact in protocol drivers
  block: Let format drivers pass @exact
  block: Pass truncate exact=true where reasonable
  Revert "qemu-img: Check post-truncation size"

 include/block/block.h  |  6 ++---
 include/block/block_int.h  | 17 -
 include/sysemu/block-backend.h |  4 +--
 block/block-backend.c  |  6 ++---
 block/commit.c |  5 ++--
 block/copy-on-read.c   |  8 --
 block/crypto.c |  8 +++---
 block/file-posix.c | 11 ++--
 block/file-win32.c |  3 ++-
 block/gluster.c|  1 +
 block/io.c | 29 -
 block/iscsi.c  | 10 ++--
 block/mirror.c |  4 +--
 block/nfs.c|  2 +-
 block/parallels.c  | 18 +++--
 block/qcow.c   |  9 ++-
 block/qcow2-refcount.c |  2 +-
 block/qcow2.c  | 45 +
 block/qed.c|  3 ++-
 block/raw-format.c |  5 ++--
 block/rbd.c|  1 +
 block/sheepdog.c   |  5 ++--
 block/ssh.c|  3 ++-
 block/vdi.c|  2 +-
 block/vhdx-log.c   |  4 +--
 block/vhdx.c   |  7 +++---
 block/vmdk.c   |  8 +++---
 block/vpc.c|  2 +-
 blockdev.c |  2 +-
 qemu-img.c | 46 --
 qemu-io-cmds.c |  7 +-
 tests/test-block-iothread.c|  8 +++---
 32 files changed, 157 insertions(+), 134 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH 1/8] block: Handle filter truncation like native impl.

[Max Reitz]

Make the filter truncation (passing it through to bs->file) a
first-class citizen and handle it exactly as if it was the filter
driver's native implementation of .bdrv_co_truncate().

I do not see a reason not to, it makes the code a bit shorter, and may
be even more correct because this gets us to finish the write_req that
we prepared before (may be important to e.g. bring dirty bitmaps to the
correct size).

Signed-off-by: Max Reitz 
---
 block/io.c | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/io.c b/block/io.c
index f8c3596131..723655c792 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3299,20 +3299,19 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, 
int64_t offset,
 goto out;
 }
 
-if (!drv->bdrv_co_truncate) {
-if (bs->file && drv->is_filter) {
-ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
-goto out;
-}
+if (drv->bdrv_co_truncate) {
+ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
+} else if (bs->file && drv->is_filter) {
+ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
+} else {
 error_setg(errp, "Image format driver does not support resize");
 ret = -ENOTSUP;
 goto out;
 }
-
-ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
 if (ret < 0) {
 goto out;
 }
+
 ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
 if (ret < 0) {
 error_setg_errno(errp, -ret, "Could not refresh total sector count");
-- 
2.21.0

[Qemu-devel] [PATCH 6/8] block: Let format drivers pass @exact

[Max Reitz]

When truncating a format node, the @exact parameter is generally handled
simply by virtue of the format storing the new size in the image
metadata.  Such formats do not need to pass on the parameter to their
file nodes.

There are exceptions, though:
- raw and crypto cannot store the image size, and thus must pass on
  @exact.

- When using qcow2 with an external data file, it just makes sense to
  keep its size in sync with the qcow2 virtual disk (because the
  external data file is the virtual disk).  Therefore, we should pass
  @exact when truncating it.

Signed-off-by: Max Reitz 
---
 block/crypto.c |  2 +-
 block/qcow2.c  | 15 ++-
 block/raw-format.c |  2 +-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/block/crypto.c b/block/crypto.c
index e5a1a2cdf3..24823835c1 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -311,7 +311,7 @@ block_crypto_co_truncate(BlockDriverState *bs, int64_t 
offset, bool exact,
 
 offset += payload_offset;
 
-return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
+return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
 }
 
 static void block_crypto_close(BlockDriverState *bs)
diff --git a/block/qcow2.c b/block/qcow2.c
index 157b9b75d9..4ef19dd29a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3822,6 +3822,13 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 if ((last_cluster + 1) * s->cluster_size < old_file_size) {
 Error *local_err = NULL;
 
+/*
+ * Do not pass @exact here: It will not help the user if
+ * we get an error here just because they wanted to shrink
+ * their qcow2 image (on a block device) with qemu-img.
+ * (And on the qcow2 layer, the @exact requirement is
+ * always fulfilled, so there is no need to pass it on.)
+ */
 bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size,
  false, PREALLOC_MODE_OFF, &local_err);
 if (local_err) {
@@ -3840,7 +3847,12 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 switch (prealloc) {
 case PREALLOC_MODE_OFF:
 if (has_data_file(bs)) {
-ret = bdrv_co_truncate(s->data_file, offset, false, prealloc, 
errp);
+/*
+ * If the caller wants an exact resize, the external data
+ * file should be resized to the exact target size, too,
+ * so we pass @exact here.
+ */
+ret = bdrv_co_truncate(s->data_file, offset, exact, prealloc, 
errp);
 if (ret < 0) {
 goto fail;
 }
@@ -3925,6 +3937,7 @@ static int coroutine_fn 
qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
 /* Allocate the data area */
 new_file_size = allocation_start +
 nb_new_data_clusters * s->cluster_size;
+/* Image file grows, so @exact does not matter */
 ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, errp);
 if (ret < 0) {
 error_prepend(errp, "Failed to resize underlying file: ");
diff --git a/block/raw-format.c b/block/raw-format.c
index 57d84d0bae..3a76ec7dd2 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -387,7 +387,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState 
*bs, int64_t offset,
 
 s->size = offset;
 offset += s->offset;
-return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
+return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
 }
 
 static void raw_eject(BlockDriverState *bs, bool eject_flag)
-- 
2.21.0

[Qemu-devel] [PATCH 3/8] block: Do not truncate file node when formatting

[Max Reitz]

There is no reason why the format drivers need to truncate the protocol
node when formatting it.  When using the old .bdrv_co_create_ops()
interface, the file will be created with no size option anyway, which
generally gives it a size of 0.  (Exceptions are block devices, which
cannot be truncated anyway.)

When using blockdev-create, the user must have given the file node some
size anyway, so there is no reason why we should override that.

qed is an exception, it needs the file to start completely empty (as
explained by c743849bee7333c7ef256b7e12e34ed6f907064f).

Signed-off-by: Max Reitz 
---
 block/parallels.c | 5 -
 block/qcow.c  | 5 -
 block/qcow2.c | 6 --
 3 files changed, 16 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index 7cd2714b69..905cac35c6 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -563,11 +563,6 @@ static int coroutine_fn 
parallels_co_create(BlockdevCreateOptions* opts,
 blk_set_allow_write_beyond_eof(blk, true);
 
 /* Create image format */
-ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
-if (ret < 0) {
-goto out;
-}
-
 bat_entries = DIV_ROUND_UP(total_size, cl_size);
 bat_sectors = DIV_ROUND_UP(bat_entry_off(bat_entries), cl_size);
 bat_sectors = (bat_sectors *  cl_size) >> BDRV_SECTOR_BITS;
diff --git a/block/qcow.c b/block/qcow.c
index 5bdf72ba33..17705015ca 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -858,11 +858,6 @@ static int coroutine_fn 
qcow_co_create(BlockdevCreateOptions *opts,
 blk_set_allow_write_beyond_eof(qcow_blk, true);
 
 /* Create image format */
-ret = blk_truncate(qcow_blk, 0, PREALLOC_MODE_OFF, errp);
-if (ret < 0) {
-goto exit;
-}
-
 memset(&header, 0, sizeof(header));
 header.magic = cpu_to_be32(QCOW_MAGIC);
 header.version = cpu_to_be32(QCOW_VERSION);
diff --git a/block/qcow2.c b/block/qcow2.c
index 4d16393e61..4978ccc7be 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3186,12 +3186,6 @@ qcow2_co_create(BlockdevCreateOptions *create_options, 
Error **errp)
 }
 blk_set_allow_write_beyond_eof(blk, true);
 
-/* Clear the protocol layer and preallocate it if necessary */
-ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
-if (ret < 0) {
-goto out;
-}
-
 /* Write the header */
 QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header));
 header = g_malloc0(cluster_size);
-- 
2.21.0

[Qemu-devel] [PATCH 2/8] block/cor: Drop cor_co_truncate()

[Max Reitz]

No other filter driver has a .bdrv_co_truncate() implementation, and
there is no need to because the general block layer code can handle it
just as well.

Signed-off-by: Max Reitz 
---
 block/copy-on-read.c | 8 
 1 file changed, 8 deletions(-)

diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 6631f30205..e95223d3cb 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -73,13 +73,6 @@ static int64_t cor_getlength(BlockDriverState *bs)
 }
 
 
-static int coroutine_fn cor_co_truncate(BlockDriverState *bs, int64_t offset,
-PreallocMode prealloc, Error **errp)
-{
-return bdrv_co_truncate(bs->file, offset, prealloc, errp);
-}
-
-
 static int coroutine_fn cor_co_preadv(BlockDriverState *bs,
   uint64_t offset, uint64_t bytes,
   QEMUIOVector *qiov, int flags)
@@ -139,7 +132,6 @@ static BlockDriver bdrv_copy_on_read = {
 .bdrv_child_perm= cor_child_perm,
 
 .bdrv_getlength = cor_getlength,
-.bdrv_co_truncate   = cor_co_truncate,
 
 .bdrv_co_preadv = cor_co_preadv,
 .bdrv_co_pwritev= cor_co_pwritev,
-- 
2.21.0

Re: [Qemu-devel] [PATCH] * include/fpu/softfloat.h (floatx80_invalid_encoding): Handle m68k specific infinity pattern.

[Alex Bennée]

Pierre Muller  writes:

>   Hi Thomas,
>
>   I tried to use git format-patch -s below,
> and change the commit message that appears below:
>
>
> muller@gcc123:~/gnu/qemu/qemu$ git format-patch -s 
> a017dc6d43aaa4ffc7be40ae3adee4086be9cec2^
> 0001-Fix-floatx80_invalid_encoding-function-for-m68k-cpu.patch
> muller@gcc123:~/gnu/qemu/qemu$ cat
> 0001-Fix-floatx80_invalid_encoding-function-for-m68k-cpu.patch

It's best to send the patches directly (i.e. don't include them in a
larger email). This is because when maintainers apply the email they end
up with a bunch of additional stuff and the corrupting of subject line.

> From a017dc6d43aaa4ffc7be40ae3adee4086be9cec2 Mon Sep 17 00:00:00 2001
> From: Pierre Muller 
> Date: Wed, 18 Sep 2019 08:04:19 +
> Subject: [PATCH]Fix floatx80_invalid_encoding function for m68k cpu
>
> As m68k accepts different patterns for infinity,
> and additional test for valid infinity must be added
> for m68k cpu target.
>
> Signed-off-by: Pierre Muller 
> ---
>  include/fpu/softfloat.h | 7 +++
>  1 file changed, 7 insertions(+)
>
> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
> index ecb8ba0114..dea24384e9 100644
> --- a/include/fpu/softfloat.h
> +++ b/include/fpu/softfloat.h
> @@ -685,10 +685,17 @@ static inline int floatx80_is_any_nan(floatx80 a)
>  | pseudo-infinities and un-normal numbers. It does not include
>  | pseudo-denormals, which must still be correctly handled as inputs even
>  | if they are never generated as outputs.
> +| As m68k accepts different patterns for infinity, thus an additional test
> +| for valid infinity value must be added for m68k CPU.
>  
> **/
>  static inline bool floatx80_invalid_encoding(floatx80 a)
>  {
> +#if defined (TARGET_M68K)
> +return ((a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0)
> +   && (! floatx80_is_infinity(a));
> +#else
>  return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
> +#endif
>  }

As most of the test is the same we could rewrite this to:

 bool invalid = (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
 #if defined (TARGET_M68K)
 invalid &= !floatx80_is_infinity(a)
 #endif
 return invalid;

The compiler should be able to simplify the logic from there.

Do we have any test cases that we could add to tests/tcg/m68k?

>
>  #define floatx80_zero make_floatx80(0x, 0xLL)


--
Alex Bennée

[Qemu-devel] [PATCH 7/8] block: Pass truncate exact=true where reasonable

[Max Reitz]

This is a change in behavior, so all instances need a good
justification.  The comments added here should explain my reasoning.

qed already had a comment that suggests it always expected
bdrv_truncate()/blk_truncate() to behave as if exact=true were passed
(c743849bee7 came eight months before 55b949c8476), so it was simply
broken until now.

Signed-off-by: Max Reitz 
---
 block/parallels.c | 11 +--
 block/qcow2.c |  6 +-
 block/qed.c   |  2 +-
 qemu-img.c|  7 ++-
 qemu-io-cmds.c|  7 ++-
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/block/parallels.c b/block/parallels.c
index a1a92c97a4..603211f83c 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -487,7 +487,12 @@ static int coroutine_fn 
parallels_co_check(BlockDriverState *bs,
 res->leaks += count;
 if (fix & BDRV_FIX_LEAKS) {
 Error *local_err = NULL;
-ret = bdrv_truncate(bs->file, res->image_end_offset, false,
+
+/*
+ * In order to really repair the image, we must shrink it.
+ * That means we have to pass exact=true.
+ */
+ret = bdrv_truncate(bs->file, res->image_end_offset, true,
 PREALLOC_MODE_OFF, &local_err);
 if (ret < 0) {
 error_report_err(local_err);
@@ -880,7 +885,9 @@ static void parallels_close(BlockDriverState *bs)
 if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) 
{
 s->header->inuse = 0;
 parallels_update_header(bs);
-bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, false,
+
+/* errors are ignored, so we might as well pass exact=true */
+bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
   PREALLOC_MODE_OFF, NULL);
 }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index 4ef19dd29a..eba165de7f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5057,7 +5057,11 @@ static int qcow2_amend_options(BlockDriverState *bs, 
QemuOpts *opts,
 return ret;
 }
 
-ret = blk_truncate(blk, new_size, false, PREALLOC_MODE_OFF, errp);
+/*
+ * Amending image options should ensure that the image has
+ * exactly the given new values, so pass exact=true here.
+ */
+ret = blk_truncate(blk, new_size, true, PREALLOC_MODE_OFF, errp);
 blk_unref(blk);
 if (ret < 0) {
 return ret;
diff --git a/block/qed.c b/block/qed.c
index 7c2a65af40..8005cfc305 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -674,7 +674,7 @@ static int coroutine_fn 
bdrv_qed_co_create(BlockdevCreateOptions *opts,
 l1_size = header.cluster_size * header.table_size;
 
 /* File must start empty and grow, check truncate is supported */
-ret = blk_truncate(blk, 0, false, PREALLOC_MODE_OFF, errp);
+ret = blk_truncate(blk, 0, true, PREALLOC_MODE_OFF, errp);
 if (ret < 0) {
 goto out;
 }
diff --git a/qemu-img.c b/qemu-img.c
index f8694f4f72..a3169b6113 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3823,7 +3823,12 @@ static int img_resize(int argc, char **argv)
 }
 }
 
-ret = blk_truncate(blk, total_size, false, prealloc, &err);
+/*
+ * The user expects the image to have the desired size after
+ * resizing, so pass @exact=true.  It is of no use to report
+ * success when the image has not actually been resized.
+ */
+ret = blk_truncate(blk, total_size, true, prealloc, &err);
 if (ret < 0) {
 error_report_err(err);
 goto out;
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 5e9017c979..1b7e700020 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1710,7 +1710,12 @@ static int truncate_f(BlockBackend *blk, int argc, char 
**argv)
 return offset;
 }
 
-ret = blk_truncate(blk, offset, false, PREALLOC_MODE_OFF, &local_err);
+/*
+ * qemu-io is a debugging tool, so let us be strict here and pass
+ * exact=true.  It is better to err on the "emit more errors" side
+ * than to be overly permissive.
+ */
+ret = blk_truncate(blk, offset, true, PREALLOC_MODE_OFF, &local_err);
 if (ret < 0) {
 error_report_err(local_err);
 return ret;
-- 
2.21.0

[Qemu-devel] [PATCH v2 1/3] audio: fix buffer-length typo in documentation

[Stefan Hajnoczi]

Fixes: f0b3d811529 ("audio: -audiodev command line option: documentation")
Signed-off-by: Stefan Hajnoczi 
---
 qemu-options.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index bbfd936d29..a4f9f74f52 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -439,7 +439,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
 "in|out.format= sample format to use with fixed settings\n"
 "valid values: s8, s16, s32, u8, u16, u32\n"
 "in|out.voices= number of voices to use\n"
-"in|out.buffer-len= length of buffer in microseconds\n"
+"in|out.buffer-length= length of buffer in microseconds\n"
 "-audiodev none,id=id,[,prop[=value][,...]]\n"
 "dummy driver that discards all output\n"
 #ifdef CONFIG_AUDIO_ALSA
@@ -524,7 +524,7 @@ Valid values are: @code{s8}, @code{s16}, @code{s32}, 
@code{u8},
 @item in|out.voices=@var{voices}
 Specify the number of @var{voices} to use.  Default is 1.
 
-@item in|out.buffer=@var{usecs}
+@item in|out.buffer-length=@var{usecs}
 Sets the size of the buffer in microseconds.
 
 @end table
-- 
2.21.0

Re: [Qemu-devel] Call for volunteers: LWN.net articles about KVM Forum talks

[Cornelia Huck]

On Tue, 17 Sep 2019 14:02:59 +0100
Stefan Hajnoczi  wrote:

> Hi,
> LWN.net is a popular open source news site that covers Linux and other
> open source communities (Python, GNOME, Debian, etc).  It has published
> a few KVM articles in the past too.
> 
> Let's raise awareness of QEMU, KVM, and libvirt by submitting articles 
> covering
> KVM Forum.

Great idea!

> 
> I am looking for ~5 volunteers who are attending KVM Forum to write an article
> about a talk they find interesting.
> 
> Please pick a talk you'd like to cover and reply to this email thread.
> I will then send an email to LWN with a heads-up so they can let us know
> if they are interested in publishing a KVM Forum special.  I will not
> ask LWN.net for money.
> 
> KVM Forum schedule:
> https://events.linuxfoundation.org/events/kvm-forum-2019/program/schedule/

I think it might make sense to cover "Managing Matryoshkas: Testing
Nested Guests" (Marc Hartmayer) and "Nesting&testing" (Vitaly
Kuznetsov) in one article, and I volunteer for that.
> 
> LWN.net guidelines:
> https://lwn.net/op/AuthorGuide.lwn
> "Our general guideline is for articles to be around 1500 words in
> length, though somewhat longer or shorter can work too. The best
> articles cover a fairly narrow topic completely, without any big
> omissions or any extra padding."
> 
> I volunteer to cover Michael Tsirkin's "VirtIO without the Virt -
> Towards Implementations in Hardware" talk.
> 
> Thanks,
> Stefan

Re: [Qemu-devel] [for-4.2 PATCH 3/6] replay: update docs for record/replay with block devices

[Pavel Dovgalyuk]

> From: Kevin Wolf [mailto:kw...@redhat.com]
> Am 18.09.2019 um 11:37 hat Pavel Dovgalyuk geschrieben:
> > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > Am 18.09.2019 um 11:22 hat Pavel Dovgalyuk geschrieben:
> > > > > From: Kevin Wolf [mailto:kw...@redhat.com]
> > > > > Am 17.09.2019 um 13:58 hat Pavel Dovgalyuk geschrieben:
> > > > > > From: Pavel Dovgalyuk 
> > > > > >
> > > > > > This patch updates the description of the command lines for using
> > > > > > record/replay with attached block devices.
> > > > > >
> > > > > > Signed-off-by: Pavel Dovgalyuk 
> > > > > > ---
> > > > > >  docs/replay.txt |   12 +---
> > > > > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/docs/replay.txt b/docs/replay.txt
> > > > > > index ee6aee9861..ce97c3f72f 100644
> > > > > > --- a/docs/replay.txt
> > > > > > +++ b/docs/replay.txt
> > > > > > @@ -27,7 +27,7 @@ Usage of the record/replay:
> > > > > >   * First, record the execution with the following command line:
> > > > > >  qemu-system-i386 \
> > > > > >   -icount shift=7,rr=record,rrfile=replay.bin \
> > > > > > - -drive file=disk.qcow2,if=none,id=img-direct \
> > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> > > > > >   -drive 
> > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> > > > > >   -device ide-hd,drive=img-blkreplay \
> > > > > >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > > > > > @@ -35,7 +35,7 @@ Usage of the record/replay:
> > > > > >   * After recording, you can replay it by using another command 
> > > > > > line:
> > > > > >  qemu-system-i386 \
> > > > > >   -icount shift=7,rr=replay,rrfile=replay.bin \
> > > > > > - -drive file=disk.qcow2,if=none,id=img-direct \
> > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct \
> > > > > >   -drive 
> > > > > > driver=blkreplay,if=none,image=img-direct,id=img-blkreplay \
> > > > > >   -device ide-hd,drive=img-blkreplay \
> > > > > >   -netdev user,id=net1 -device rtl8139,netdev=net1 \
> > > > > > @@ -223,7 +223,7 @@ Block devices record/replay module intercepts 
> > > > > > calls of
> > > > > >  bdrv coroutine functions at the top of block drivers stack.
> > > > > >  To record and replay block operations the drive must be configured
> > > > > >  as following:
> > > > > > - -drive file=disk.qcow2,if=none,id=img-direct
> > > > > > + -drive file=disk.qcow2,if=none,snapshot,id=img-direct
> > > > > >   -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > > > > >   -device ide-hd,drive=img-blkreplay
> > > > > >
> > > > > > @@ -252,6 +252,12 @@ This snapshot is created at start of recording 
> > > > > > and restored at
> > > start
> > > > > >  of replaying. It also can be loaded while replaying to roll back
> > > > > >  the execution.
> > > > > >
> > > > > > +'snapshot' flag of the disk image must be removed to save the 
> > > > > > snapshots
> > > > > > +in the overlay (or original image) instead of using the temporary 
> > > > > > overlay.
> > > > > > + -drive file=disk.ovl,if=none,id=img-direct
> > > > > > + -drive driver=blkreplay,if=none,image=img-direct,id=img-blkreplay
> > > > > > + -device ide-hd,drive=img-blkreplay
> > > > >
> > > > > Wait, didn't patch 2 just make -snapshot unconditionally incompatible
> > > > > with replay? So isn't saving the snapshot in the original image the 
> > > > > only
> > > > > supported mode now?
> > > >
> > > > There are two ways to run record/replay:
> > > > 1. Disk with snapshot option and any image to make it unchanged
> > > > 2. Disk with overlay without snapshot option to save VM snapshots on it
> > >
> > > Yes, I think I understand the two options that you intend to make
> > > available, but when -snapshot sets a replay blocker, how can 1. still
> > > work? Is there some code that ignores replay blockers?
> >
> > I checked the text and don't see anything about global "-snapshot" option.
> > All references are about drive snapshot flag.
> > Can you point me where did I missed that?
> 
> Oh, sorry, you're right and I misread.
> 
> However, global -snapshot is just a convenient shortcut for specifying
> snapshot=on for all -drive arguments. So if -snapshot is incompatible
> with replay, shouldn't manually marking all drives as snapshot=on be
> incompatible as well?
> 
> Maybe you're really interested in some specific drive not having
> snapshot=on? But then it might be better to check that specific drive
> instad of forbidding just the shortcut for setting it.

-snapshot adds the flag for top-level drive, making driver operations
dependent on temporary file structure.

Moving this overlay beneath blkreplay driver makes drive operations
deterministic for the top-level device.

Pavel Dovgalyuk

[Qemu-devel] [PATCH v2 0/3] audio: -audiodev documentation tweaks

[Stefan Hajnoczi]

v2:
 * Added ALSA period-length fix [Zoltán]
 * Expanded PulseAudio latency documentation [Zoltán]

Small fixes to the -audiodev documentation.

Stefan Hajnoczi (3):
  audio: fix buffer-length typo in documentation
  audio: add -audiodev pa,in|out.latency= to documentation
  audio: fix ALSA period-length typo in documentation

 qemu-options.hx | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH 8/8] Revert "qemu-img: Check post-truncation size"

[Max Reitz]

This reverts commit 5279b30392da7a3248b320c75f20c61e3a95863c.

We no longer need this check because exact=true forces the block driver
to give the image the exact size requested by the user.

Signed-off-by: Max Reitz 
---
 qemu-img.c | 39 ---
 1 file changed, 4 insertions(+), 35 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index a3169b6113..148f6b8b0e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3648,7 +3648,7 @@ static int img_resize(int argc, char **argv)
 Error *err = NULL;
 int c, ret, relative;
 const char *filename, *fmt, *size;
-int64_t n, total_size, current_size, new_size;
+int64_t n, total_size, current_size;
 bool quiet = false;
 BlockBackend *blk = NULL;
 PreallocMode prealloc = PREALLOC_MODE_OFF;
@@ -3829,42 +3829,11 @@ static int img_resize(int argc, char **argv)
  * success when the image has not actually been resized.
  */
 ret = blk_truncate(blk, total_size, true, prealloc, &err);
-if (ret < 0) {
+if (!ret) {
+qprintf(quiet, "Image resized.\n");
+} else {
 error_report_err(err);
-goto out;
-}
-
-new_size = blk_getlength(blk);
-if (new_size < 0) {
-error_report("Failed to verify truncated image length: %s",
- strerror(-new_size));
-ret = -1;
-goto out;
 }
-
-/* Some block drivers implement a truncation method, but only so
- * the user can cause qemu to refresh the image's size from disk.
- * The idea is that the user resizes the image outside of qemu and
- * then invokes block_resize to inform qemu about it.
- * (This includes iscsi and file-posix for device files.)
- * Of course, that is not the behavior someone invoking
- * qemu-img resize would find useful, so we catch that behavior
- * here and tell the user. */
-if (new_size != total_size && new_size == current_size) {
-error_report("Image was not resized; resizing may not be supported "
- "for this image");
-ret = -1;
-goto out;
-}
-
-if (new_size != total_size) {
-warn_report("Image should have been resized to %" PRIi64
-" bytes, but was resized to %" PRIi64 " bytes",
-total_size, new_size);
-}
-
-qprintf(quiet, "Image resized.\n");
-
 out:
 blk_unref(blk);
 if (ret) {
-- 
2.21.0

Re: [Qemu-devel] [libvirt] Call for volunteers: LWN.net articles about KVM Forum talks

[Stefan Hajnoczi]

On Wed, Sep 18, 2019 at 9:28 AM Stefano Garzarella  wrote:
>
> On Tue, Sep 17, 2019 at 02:02:59PM +0100, Stefan Hajnoczi wrote:
> I volunteer for "Libvirt: Never too Late to Learn New Tricks" by
> Daniel Berrange.

Hi Stefano,
Paolo has already volunteered for that.  Is there another talk you are
interested in covering?

Stefan

[Qemu-devel] [PATCH v2 3/3] audio: fix ALSA period-length typo in documentation

[Stefan Hajnoczi]

Fixes: f0b3d811529 ("audio: -audiodev command line option: documentation")
Signed-off-by: Stefan Hajnoczi 
---
 qemu-options.hx | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 6d7fe57dd4..0f79c70c37 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -445,7 +445,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
 #ifdef CONFIG_AUDIO_ALSA
 "-audiodev alsa,id=id[,prop[=value][,...]]\n"
 "in|out.dev= name of the audio device to use\n"
-"in|out.period-len= length of period in microseconds\n"
+"in|out.period-length= length of period in microseconds\n"
 "in|out.try-poll= attempt to use poll mode\n"
 "threshold= threshold (in microseconds) when playback 
starts\n"
 #endif
@@ -546,7 +546,7 @@ ALSA specific options are:
 Specify the ALSA @var{device} to use for input and/or output.  Default
 is @code{default}.
 
-@item in|out.period-len=@var{usecs}
+@item in|out.period-length=@var{usecs}
 Sets the period length in microseconds.
 
 @item in|out.try-poll=on|off
-- 
2.21.0

Re: [Qemu-devel] [PATCH v3 3/3] virtio: add vhost-user-fs-pci device

[Stefan Hajnoczi]

On Wed, Sep 18, 2019 at 09:27:21AM +0800, piaojun wrote:
> 
> 
> On 2019/9/18 0:00, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Add the PCI version of vhost-user-fs.
> > 
> > Launch QEMU like this:
> > 
> >   qemu -chardev socket,path=/tmp/vhost-fs.sock,id=chr0
> >-device x-vhost-user-fs-pci,tag=myfs,chardev=chr0
> > 
> > Signed-off-by: Stefan Hajnoczi 
> > Signed-off-by: Sebastien Boeuf 
> > Signed-off-by: Dr. David Alan Gilbert 
> > Reviewed-by: Cornelia Huck 
> > ---
> >  hw/virtio/Makefile.objs   |  1 +
> >  hw/virtio/vhost-user-fs-pci.c | 85 +++
> >  2 files changed, 86 insertions(+)
> >  create mode 100644 hw/virtio/vhost-user-fs-pci.c
> > 
> > diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
> > index 47ffbf22c4..e2f70fbb89 100644
> > --- a/hw/virtio/Makefile.objs
> > +++ b/hw/virtio/Makefile.objs
> > @@ -15,6 +15,7 @@ obj-$(CONFIG_VHOST_USER_FS) += vhost-user-fs.o
> >  obj-$(call land,$(CONFIG_VIRTIO_CRYPTO),$(CONFIG_VIRTIO_PCI)) += 
> > virtio-crypto-pci.o
> >  obj-$(CONFIG_VIRTIO_PMEM) += virtio-pmem.o
> >  common-obj-$(call land,$(CONFIG_VIRTIO_PMEM),$(CONFIG_VIRTIO_PCI)) += 
> > virtio-pmem-pci.o
> > +obj-$(call land,$(CONFIG_VHOST_USER_FS),$(CONFIG_VIRTIO_PCI)) += 
> > vhost-user-fs-pci.o
> >  obj-$(CONFIG_VHOST_VSOCK) += vhost-vsock.o
> >  
> >  ifeq ($(CONFIG_VIRTIO_PCI),y)
> > diff --git a/hw/virtio/vhost-user-fs-pci.c b/hw/virtio/vhost-user-fs-pci.c
> > new file mode 100644
> > index 00..0e70985094
> > --- /dev/null
> > +++ b/hw/virtio/vhost-user-fs-pci.c
> > @@ -0,0 +1,85 @@
> > +/*
> > + * Vhost-user filesystem virtio device PCI glue
> > + *
> > + * Copyright 2018-2019 Red Hat, Inc.
> > + *
> > + * Authors:
> > + *  Dr. David Alan Gilbert 
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > + * (at your option) any later version.  See the COPYING file in the
> > + * top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "hw/qdev-properties.h"
> > +#include "hw/virtio/vhost-user-fs.h"
> > +#include "virtio-pci.h"
> > +
> > +struct VHostUserFSPCI {
> > +VirtIOPCIProxy parent_obj;
> > +VHostUserFS vdev;
> > +};
> > +
> > +typedef struct VHostUserFSPCI VHostUserFSPCI;
> > +
> > +#define TYPE_VHOST_USER_FS_PCI "vhost-user-fs-pci-base"
> > +
> > +#define VHOST_USER_FS_PCI(obj) \
> > +OBJECT_CHECK(VHostUserFSPCI, (obj), TYPE_VHOST_USER_FS_PCI)
> > +
> > +static Property vhost_user_fs_pci_properties[] = {
> > +DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
> > +   DEV_NVECTORS_UNSPECIFIED),
> > +DEFINE_PROP_END_OF_LIST(),
> > +};
> > +
> > +static void vhost_user_fs_pci_realize(VirtIOPCIProxy *vpci_dev, Error 
> > **errp)
> > +{
> > +VHostUserFSPCI *dev = VHOST_USER_FS_PCI(vpci_dev);
> > +DeviceState *vdev = DEVICE(&dev->vdev);
> > +
> > +if (vpci_dev->nvectors == DEV_NVECTORS_UNSPECIFIED) {
> > +vpci_dev->nvectors = dev->vdev.conf.num_request_queues + 1;
> > +}
> > +
> > +qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
> > +object_property_set_bool(OBJECT(vdev), true, "realized", errp);
> > +}
> > +
> > +static void vhost_user_fs_pci_class_init(ObjectClass *klass, void *data)
> > +{
> > +DeviceClass *dc = DEVICE_CLASS(klass);
> > +VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
> > +PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
> > +k->realize = vhost_user_fs_pci_realize;
> > +set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> > +dc->props = vhost_user_fs_pci_properties;
> > +pcidev_k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
> > +pcidev_k->device_id = 0; /* Set by virtio-pci based on virtio id */
> 
> Why not set *PCI_DEVICE_ID_VIRTIO_FS* here just like virtio_blk? I'm
> not very familiar with this code, and just compare it with the other
> same logic.

The comment indicates that virtio-pci computes the correct PCI Device ID
based on the VIRTIO Device ID.  The PCI Device ID depends on the
Legacy/Transitional/Modern mode supported by the device and therefore
cannot be hardcoded.  If we set it here it will be overwritten later
anyway.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 29/29] tests/tcg: target/s390x: Test MVO

[David Hildenbrand]

On 18.09.19 11:47, Alex Bennée wrote:
> 
> David Hildenbrand  writes:
> 
>> Let's add the simple test based on the example from the PoP.
>>
>> Signed-off-by: David Hildenbrand 
>> ---
>>  tests/tcg/s390x/Makefile.target |  1 +
>>  tests/tcg/s390x/mvo.c   | 25 +
>>  2 files changed, 26 insertions(+)
>>  create mode 100644 tests/tcg/s390x/mvo.c
>>
>> diff --git a/tests/tcg/s390x/Makefile.target 
>> b/tests/tcg/s390x/Makefile.target
>> index 151dc075aa..6a3bfa8b29 100644
>> --- a/tests/tcg/s390x/Makefile.target
>> +++ b/tests/tcg/s390x/Makefile.target
>> @@ -6,3 +6,4 @@ TESTS+=ipm
>>  TESTS+=exrl-trt
>>  TESTS+=exrl-trtr
>>  TESTS+=pack
>> +TESTS+=mvo
>> diff --git a/tests/tcg/s390x/mvo.c b/tests/tcg/s390x/mvo.c
>> new file mode 100644
>> index 00..5546fe2a97
>> --- /dev/null
>> +++ b/tests/tcg/s390x/mvo.c
>> @@ -0,0 +1,25 @@
>> +#include 
>> +#include 
>> +
>> +int main(void)
>> +{
>> +uint8_t dest[6] = {0xff, 0x77, 0x88, 0x99, 0x0c, 0xff};
>> +uint8_t src[5] = {0xee, 0x12, 0x34, 0x56, 0xee};
>> +uint8_t expected[6] = {0xff, 0x01, 0x23, 0x45, 0x6c, 0xff};
>> +int i;
>> +
>> +asm volatile (
>> +"mvo 0(4,%[dest]),0(3,%[src])\n"
>> +:
>> +: [dest] "d" (dest + 1),
>> +  [src] "d" (src + 1)
>> +: "memory");
>> +
>> +for (i = 0; i < sizeof(expected); i++) {
>> +if (dest[i] != expected[i]) {
>> +fprintf(stderr, "bad data\n");
>> +return 1;
>> +}
>> +}
>> +return 0;
>> +}
> 
> Reviewed-by: Alex Bennée 
> 
> but...
> 
> can this test be expanded to check the page cross cases that caused you
> so much trouble to track down?

I might add a MVC test that tries to reproduce this. But with
speculative page faults and things like that it might not be very easy
to reproduce. However, I can give it a try.

Thanks!

> 
> --
> Alex Bennée
> 


-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH v2 2/3] audio: add -audiodev pa, in|out.latency= to documentation

[Stefan Hajnoczi]

The "latency" parameter wasn't covered by the documentation.

Signed-off-by: Stefan Hajnoczi 
---
 qemu-options.hx | 5 +
 1 file changed, 5 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index a4f9f74f52..6d7fe57dd4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -470,6 +470,7 @@ DEF("audiodev", HAS_ARG, QEMU_OPTION_audiodev,
 "-audiodev pa,id=id[,prop[=value][,...]]\n"
 "server= PulseAudio server address\n"
 "in|out.name= source/sink device name\n"
+"in|out.latency= desired latency in microseconds\n"
 #endif
 #ifdef CONFIG_AUDIO_SDL
 "-audiodev sdl,id=id[,prop[=value][,...]]\n"
@@ -630,6 +631,10 @@ Sets the PulseAudio @var{server} to connect to.
 @item in|out.name=@var{sink}
 Use the specified source/sink for recording/playback.
 
+@item in|out.latency=@var{usecs}
+Desired latency in microseconds.  The PulseAudio server will try to honor this
+value but actual latencies may be lower or higher.
+
 @end table
 
 @item -audiodev sdl,id=@var{id}[,@var{prop}[=@var{value}][,...]]
-- 
2.21.0

[Qemu-devel] [PATCH] Fix wrong behavior of cpu_memory_rw_debug() function in SMM

[Dmitry Poletaev]

There is a problem, that you don't have access to the data using 
cpu_memory_rw_debug() function when in SMM. You can't remotely debug SMM mode 
program because of that for example.
Likely attrs version of get_phys_page_debug should be used to get correct asidx 
at the end to handle access properly.
Here the patch to fix it.

Signed-off-by: Dmitry Poletaev 
---
 target/i386/cpu.c| 2 +-
 target/i386/cpu.h| 3 ++-
 target/i386/helper.c | 5 -
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 9e0bac31e8..8ade4ed2c6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5984,7 +5984,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, 
void *data)
 #ifndef CONFIG_USER_ONLY
 cc->asidx_from_attrs = x86_asidx_from_attrs;
 cc->get_memory_mapping = x86_cpu_get_memory_mapping;
-cc->get_phys_page_debug = x86_cpu_get_phys_page_debug;
+cc->get_phys_page_attrs_debug = x86_cpu_get_phys_page_attrs_debug;
 cc->write_elf64_note = x86_cpu_write_elf64_note;
 cc->write_elf64_qemunote = x86_cpu_write_elf64_qemunote;
 cc->write_elf32_note = x86_cpu_write_elf32_note;
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 5f6e3a029a..bbd00d8deb 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1549,7 +1549,8 @@ void x86_cpu_get_memory_mapping(CPUState *cpu, 
MemoryMappingList *list,
 
 void x86_cpu_dump_state(CPUState *cs, FILE *f, int flags);
 
-hwaddr x86_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
+hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cpu, vaddr addr,
+ MemTxAttrs *attrs);
 
 int x86_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int x86_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
diff --git a/target/i386/helper.c b/target/i386/helper.c
index 0fa51be646..c3a6e4fabe 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -715,7 +715,8 @@ void cpu_x86_update_cr4(CPUX86State *env, uint32_t new_cr4)
 }
 
 #if !defined(CONFIG_USER_ONLY)
-hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
+hwaddr x86_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
+ MemTxAttrs *attrs)
 {
 X86CPU *cpu = X86_CPU(cs);
 CPUX86State *env = &cpu->env;
@@ -725,6 +726,8 @@ hwaddr x86_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
 uint32_t page_offset;
 int page_size;
 
+*attrs = cpu_get_mem_attrs(env);
+
 a20_mask = x86_get_a20_mask(env);
 if (!(env->cr[0] & CR0_PG_MASK)) {
 pte = addr & a20_mask;
-- 
2.11.0

Re: [Qemu-devel] [PATCH] * include/fpu/softfloat.h (floatx80_invalid_encoding): Handle m68k specific infinity pattern.

[Laurent Vivier]

Le 18/09/2019 à 11:59, Alex Bennée a écrit :
> 
> Pierre Muller  writes:
> 
>>   Hi Thomas,
>>
>>   I tried to use git format-patch -s below,
>> and change the commit message that appears below:
>>
>>
>> muller@gcc123:~/gnu/qemu/qemu$ git format-patch -s 
>> a017dc6d43aaa4ffc7be40ae3adee4086be9cec2^
>> 0001-Fix-floatx80_invalid_encoding-function-for-m68k-cpu.patch
>> muller@gcc123:~/gnu/qemu/qemu$ cat
>> 0001-Fix-floatx80_invalid_encoding-function-for-m68k-cpu.patch
> 
> It's best to send the patches directly (i.e. don't include them in a
> larger email). This is because when maintainers apply the email they end
> up with a bunch of additional stuff and the corrupting of subject line.
> 
>> From a017dc6d43aaa4ffc7be40ae3adee4086be9cec2 Mon Sep 17 00:00:00 2001
>> From: Pierre Muller 
>> Date: Wed, 18 Sep 2019 08:04:19 +
>> Subject: [PATCH]Fix floatx80_invalid_encoding function for m68k cpu
>>
>> As m68k accepts different patterns for infinity,
>> and additional test for valid infinity must be added
>> for m68k cpu target.
>>
>> Signed-off-by: Pierre Muller 
>> ---
>>  include/fpu/softfloat.h | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
>> index ecb8ba0114..dea24384e9 100644
>> --- a/include/fpu/softfloat.h
>> +++ b/include/fpu/softfloat.h
>> @@ -685,10 +685,17 @@ static inline int floatx80_is_any_nan(floatx80 a)
>>  | pseudo-infinities and un-normal numbers. It does not include
>>  | pseudo-denormals, which must still be correctly handled as inputs even
>>  | if they are never generated as outputs.
>> +| As m68k accepts different patterns for infinity, thus an additional test
>> +| for valid infinity value must be added for m68k CPU.
>>  
>> **/
>>  static inline bool floatx80_invalid_encoding(floatx80 a)
>>  {
>> +#if defined (TARGET_M68K)
>> +return ((a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0)
>> +   && (! floatx80_is_infinity(a));
>> +#else
>>  return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
>> +#endif
>>  }
> 
> As most of the test is the same we could rewrite this to:
> 
>  bool invalid = (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
>  #if defined (TARGET_M68K)
>  invalid &= !floatx80_is_infinity(a)
>  #endif
>  return invalid;
> 
> The compiler should be able to simplify the logic from there.
> 
> Do we have any test cases that we could add to tests/tcg/m68k?
> 
>>
>>  #define floatx80_zero make_floatx80(0x, 0xLL)
> 

I think patch from Pierre doesn't treat all the problem and don't rely 
on any specification.

I proposed a patch years ago that is closer to the hardware behaviour:

/*
| Return whether the given value is an invalid floatx80 encoding.
| Invalid floatx80 encodings arise when the integer bit is not set, but
| the exponent is not zero. The only times the integer bit is permitted to
| be zero is in subnormal numbers and the value zero.
| This includes what the Intel software developer's manual calls pseudo-NaNs,
| pseudo-infinities and un-normal numbers. It does not include
| pseudo-denormals, which must still be correctly handled as inputs even
| if they are never generated as outputs.
**/
static inline bool floatx80_invalid_encoding(floatx80 a)
{
#if defined(TARGET_M68K)
/*-
|  M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL
|  1.6.2 Denormalized Numbers
|  Since the extended-precision data format has an explicit integer bit,
|  a number can be formatted with a nonzero exponent, less than the maximum
|  value, and a zero integer bit.  The IEEE 754 standard does not define a
|  zero integer bit. Such a number is an unnormalized number. Hardware does
|  not directly support denormalized and unnormalized numbers, but
|  implicitly supports them by trapping them as unimplemented data types,
|  allowing efficient conversion in software.
**/
return 0;
#else
return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0;
#endif
}

But in fact I think this kind of number should raise an exception.

I tried to do this in:

https://github.com/vivier/qemu-m68k/commit/5bd7742437a3c770373734add5ab452e8df4e621

but it needs more work and for the moment doesn't fix the problem Pierre is 
trying to fix.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH 02/17] block: Pass local error object pointer to error_append_hint()

[Vladimir Sementsov-Ogievskiy]

18.09.2019 10:58, Greg Kurz wrote:
> On Tue, 17 Sep 2019 17:40:11 +
> Vladimir Sementsov-Ogievskiy  wrote:
> 
>> 17.09.2019 18:37, Greg Kurz wrote:
>>> On Tue, 17 Sep 2019 13:25:03 +
>>> Vladimir Sementsov-Ogievskiy  wrote:
>>>
 17.09.2019 13:20, Greg Kurz wrote:
> Ensure that hints are added even if errp is &error_fatal or &error_abort.
>
> Signed-off-by: Greg Kurz 
> ---
> block/backup.c   |7 +--
> block/dirty-bitmap.c |7 +--
> block/file-posix.c   |   20 +---
> block/gluster.c  |   23 +++
> block/qcow.c |   10 ++
> block/qcow2.c|7 +--
> block/vhdx-log.c |7 +--
> block/vpc.c  |7 +--
> 8 files changed, 59 insertions(+), 29 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index 763f0d7ff6db..d8c422a0e3bc 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -602,11 +602,14 @@ static int64_t 
> backup_calculate_cluster_size(BlockDriverState *target,
> BACKUP_CLUSTER_SIZE_DEFAULT);
> return BACKUP_CLUSTER_SIZE_DEFAULT;
> } else if (ret < 0 && !target->backing) {
> -error_setg_errno(errp, -ret,
> +Error *local_err = NULL;
> +
> +error_setg_errno(&local_err, -ret,
> "Couldn't determine the cluster size of the target image, 
> "
> "which has no backing file");
> -error_append_hint(errp,
> +error_append_hint(&local_err,
> "Aborting, since this may create an unusable destination 
> image\n");
> +error_propagate(errp, local_err);
> return ret;
> } else if (ret < 0 && target->backing) {
> /* Not fatal; just trudge on ahead. */


 Pain.. Do we need these hints, if they are so painful?

>>>
>>> I agree that the one above doesn't qualify as a useful hint.
>>> It just tells that QEMU is giving up and gives no indication
>>> to the user on how to avoid the issue. It should probably be
>>> dropped.
>>>
 At least for cases like this, we can create helper function

 error_setg_errno_hint(..., error, hint)
>>>
>>> Not very useful if hint has to be forged separately with
>>> g_sprintf(), and we can't have such a thing as:
>>>
>>> error_setg_errno_hint(errp, err_fmt, ..., hint_fmt, ...)
>>>

 But what could be done when we call function, which may or may not set 
 errp?

 ret = f(errp);
 if (ret) {
   error_append_hint(errp, hint);
 }

>>>
>>> Same problem. If errp is &error_fatal and f() does errno_setg(errp), it
>>> ends up calling exit().
>>>
 Hmmm..

 Can it look like

 ret = f(..., hint_helper(errp, hint))

 ?

>>>
>>> Nope, hint_helper() will get called before f() and things are worse.
>>> If errp is NULL then error_append_hint() does nothing and if it is
>>> &error_fatal then it aborts.
>>>
 I can't imagine how to do it, as someone should remove hint from 
 error_abort object on
 success path..

 But seems, the following is possible, which seems better for me than 
 local-error approach:

 error_push_hint(errp, hint);
 ret = f(.., errp);
 error_pop_hint(errp);

>>>
>>> Matter of taste... also, it looks awkward to come up with a hint
>>> before knowing what happened. I mean the appropriate hint could
>>> depend on the value returned by f() and/or errno for example.
>>>
 ===

 Continue thinking on this:

 It may look like just
 ret = f(..., set_hint(errp, hint));

 or (just to split long line):
 set_hint(errp, hint);
 ret = f(..., errp);

 if in each function with errp does error_push_hint(errp) on start and 
 error_pop_hint(errp) on exit,
 which may be just one call at function start of macro, which will call 
 error_push_hint(errp) and
 define local variable by g_auto, with cleanup which will call 
 error_pop_hint(errp) on function
 exit..

 Or, may be, more direct way to set cleanup for function exists?

 ===

 Also, we can implement some code generation, to generate for functions 
 with errp argument
 wrappers with additional hint parameter, and just use these wrappers..

 ===

 If nobody likes any of my suggestions, then ignore them. I understand that 
 this series fixes
 real issue and much effort has already been spent. May be one day I'll try 
 to rewrite it...

>>>
>>> For the reason exposed above, I don't think it makes sense to
>>> build the hint before calling a function that calls error_setg().
>>> I'm afraid we're stuck with local_err... it is then up to the
>>> people to make it as less painful as possible.
>>>
>>
>> Hmm. so, seem

Re: [Qemu-devel] [PATCH 10/17] s390x/css: Pass local error object pointer to error_append_hint()

[Cornelia Huck]

On Tue, 17 Sep 2019 18:36:20 +0200
Greg Kurz  wrote:

> On Tue, 17 Sep 2019 13:24:12 +0200
> Cornelia Huck  wrote:
> 
> > On Tue, 17 Sep 2019 12:21:34 +0200
> > Greg Kurz  wrote:
> >   
> > > Ensure that hints are added even if errp is &error_fatal or &error_abort.
> > > 
> > > Signed-off-by: Greg Kurz 
> > > ---
> > >  hw/s390x/s390-ccw.c |6 --
> > >  1 file changed, 4 insertions(+), 2 deletions(-)  
> > 
> > Reviewed-by: Cornelia Huck 
> > 
> > Can also take this via the s390 tree, let me know what would work best.  
> 
> I guess it would be easier to merge if each individual patch goes
> through the corresponding sub-maintainer tree. But Eric mentioned
> in another mail that the whole whole series could maybe go through
> Markus' error tree... so, I don't know which is best. :)

Ok, it's probably best to take this through the s390 tree, as I plan to
send a pull request tomorrow :)

Thanks, applied.

Re: [Qemu-devel] [PATCH] * include/fpu/softfloat.h (floatx80_invalid_encoding): Handle m68k specific infinity pattern.

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/1615bbe5-3033-3b76-5cfb-52e343dc4...@freepascal.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH] * include/fpu/softfloat.h 
(floatx80_invalid_encoding): Handle m68k specific infinity pattern.
Message-id: 1615bbe5-3033-3b76-5cfb-52e343dc4...@freepascal.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
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20190918095335.7646-1-stefa...@redhat.com -> 
patchew/20190918095335.7646-1-stefa...@redhat.com
 * [new tag] patchew/20190918100706.19753-1-polet...@ispras.ru -> 
patchew/20190918100706.19753-1-polet...@ispras.ru
Switched to a new branch 'test'
0e0d352 * include/fpu/softfloat.h (floatx80_invalid_encoding): Handle m68k 
specific infinity pattern.

=== OUTPUT BEGIN ===
ERROR: space prohibited between function name and open parenthesis '('
#48: FILE: include/fpu/softfloat.h:693:
+#if defined (TARGET_M68K)

ERROR: space prohibited after that '!' (ctx:BxW)
#50: FILE: include/fpu/softfloat.h:695:
+   && (! floatx80_is_infinity(a));
^

ERROR: Missing Signed-off-by: line(s)

total: 3 errors, 0 warnings, 17 lines checked

Commit 0e0d352d93d2 (* include/fpu/softfloat.h (floatx80_invalid_encoding): 
Handle m68k specific infinity pattern.) has style problems, please review.  If 
any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/1615bbe5-3033-3b76-5cfb-52e343dc4...@freepascal.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [libvirt] Call for volunteers: LWN.net articles about KVM Forum talks

[Stefano Garzarella]

On Wed, Sep 18, 2019 at 11:02:54AM +0100, Stefan Hajnoczi wrote:
> On Wed, Sep 18, 2019 at 9:28 AM Stefano Garzarella  
> wrote:
> >
> > On Tue, Sep 17, 2019 at 02:02:59PM +0100, Stefan Hajnoczi wrote:
> > I volunteer for "Libvirt: Never too Late to Learn New Tricks" by
> > Daniel Berrange.
> 
> Hi Stefano,
> Paolo has already volunteered for that.  Is there another talk you are
> interested in covering?

Hi Stefan,
another talk I'm interested in is "Making the Most of NBD" by Eric Blake &
Richard Jones.

I hope it's not already covered :-)

Cheers,
Stefano

Re: [Qemu-devel] [PATCH v4 1/4] iotests: add script_initialize

[Vladimir Sementsov-Ogievskiy]

18.09.2019 1:29, John Snow wrote:
> 
> 
> On 9/16/19 10:56 AM, Vladimir Sementsov-Ogievskiy wrote:
>> 12.09.2019 3:16, John Snow wrote:
>>> Like script_main, but doesn't require a single point of entry.
>>> Replace all existing initialization sections with this drop-in replacement.
>>>
>>> This brings debug support to all existing script-style iotests.
>>>
>>> Note: supported_oses=['linux'] was omitted, as it is a default argument.
>>
>> But after this patch all test which didn't check os start to check linux
>> (as it's default).. So all tests which worked on other platforms will now
>> be skipped on these other platforms?
>>
>> Finally do we support something except linux for iotests?
>> for bash tests _supported_os also used only with "Linux" in 87 tests..
>>
>> May be we'd better drop both _supported_os and supported_oses alltogether,
>> and don't care?
>>
>> Anyway, if we support only linux, any reason to skip almost all tests,
>> if someone tries to run test on other platform? Let him run what he wants.
>>
> 
> Currently, the following tests are python:
> 
> 030 040 041 044 045 055 056 057 065 093 096 118 124 129 132 136 139 147
> 148 149 151 152 155 163 165 169 194 196 199 202 203 205 206 207 208 209
> 210 211 212 213 216 218 219 222 224 228 234 235 236 237 238 242 245 246
> 248 254 255 256 257 258 262 266
> 
> And as it stands, none of the script-style python tests we've selected
> to run in `make check` fail on the FreeBSD platform.
> 
> So as an experiment, I lifted the restriction on python tests. I kept
> running ./check until I found some invocation that they didn't skip.
> 
> Failures: 045 147 149 169 194 199 211
> 
> Not too bad...
> 
> 045: +qemu.machine.QEMUMachineError: socket_scm_helper does not exist
> 149: Wants to use 'sudo', but our freebsd image doesn't have that.
> 194: +OSError: AF_UNIX path too long
> 211:
> -[{ "start": 0, "length": 3072, "depth": 0, "zero": false, "data": true,
> "offset": 1024},
> -{ "start": 3072, "length": 33551360, "depth": 0, "zero": true, "data":
> true, "offset": 4096}]
> +[{ "start": 0, "length": 31744, "depth": 0, "zero": false, "data":
> true, "offset": 1024},
> +{ "start": 31744, "length": 33522688, "depth": 0, "zero": true, "data":
> true, "offset": 32768}]
> 
> 
> 149 can probably be fixed, and 194 and 211 I have fail in similar ways
> on my own Linux machine, so that's probably not BSD's fault.
> 
> Interestingly, 169 and 199, bitmap migration related tests, cause a
> SIGSEGV in QEMU ...
> 
> 
> 169:
> +
> +WARNING:qemu.machine:qemu received signal 6:
> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
> -chardev
> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmprpc0idbx/qemub-26617-monitor.sock
> -mon chardev=mon,mode=control -display none -vga none -qtest
> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-26617-qtest.sock
> -machine accel=qtest -nodefaults -display none -machine accel=qtest
> -incoming defer -drive
> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=writeback
> 
> The common thread in 169 is the +migbitmap trait, which ... makes me a
> little nervous, of course!
> 
> 
> 199:
> +WARNING:qemu.machine:qemu received signal 6:
> /usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64
> -chardev
> socket,id=mon,path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/tmpvzpyc9j6/qemub-30170-monitor.sock
> -mon chardev=mon,mode=control -display none -vga none -qtest
> unix:path=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/qemub-30170-qtest.sock
> -machine accel=qtest -nodefaults -display none -machine accel=qtest
> -drive
> if=virtio,id=drive0,file=/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/disk_b,format=qcow2,cache=none
> -incoming exec: cat
> '/usr/home/qemu/qemu-test.IfsR68/build/tests/qemu-iotests/scratch/mig_fifo'
> +E
> 
> 
> Vladimir, I was able to provoke this error by editing
> ./tests/vm/Makefile.include and removing the --snapshot invocation, then
> using `make vm-build-freebsd` and finally typing `make vm-ssh-freebsd`
> to open up a shell.
> 
> Then the tricks are the usual ones; navigate to iotests directory,
> ./check -v -qcow2 169, etc. You'll need to create a build that allows
> Python tests to run; do it before you run the snapshot-less build.
> 
> 

Interesting, I'll try to reproduce.

> 
> 
> aaand lastly, running `make check` doesn't happen to call any of the
> tests that appear broken on FreeBSD right now, so I'm just going to go
> ahead and say we can open Pandora's box and make the default python test
> behavior to run on any OS, and start re-blacklisting the edge-cases as
> we find them.
> 
> As far as iotests go, there are a few new broken ones that surface, but
> they won't gate Peter Maydell's build process because

Re: [Qemu-devel] [for-4.2 PATCH 0/6] Block-related record/replay fixes

[Alex Bennée]

Pavel Dovgalyuk  writes:

>> -Original Message-
>> From: Alex Bennée [mailto:alex.ben...@linaro.org]
>> Sent: Tuesday, September 17, 2019 10:02 PM
>> To: Pavel Dovgalyuk
>> Cc: qemu-devel@nongnu.org; kw...@redhat.com; peter.mayd...@linaro.org;
>> crosthwaite.pe...@gmail.com; boost.li...@gmail.com; 
>> artem.k.pisare...@gmail.com;
>> quint...@redhat.com; ciro.santi...@gmail.com; jasow...@redhat.com; 
>> m...@redhat.com;
>> arm...@redhat.com; mre...@redhat.com; maria.klimushenk...@ispras.ru; 
>> dovga...@ispras.ru;
>> kra...@redhat.com; pavel.dovga...@ispras.ru; thomas.dull...@googlemail.com;
>> pbonz...@redhat.com; dgilb...@redhat.com; r...@twiddle.net
>> Subject: Re: [for-4.2 PATCH 0/6] Block-related record/replay fixes
>>
>>
>> Pavel Dovgalyuk  writes:
>>
>> > The set of patches include the block-related updates
>> > of the record/replay icount feature:
>> >  - application of 'snapshot' option on the file layer instead of
>> >the top one: command line and documentation fix
>> >  - implementation of bdrv_snapshot_goto for blkreplay driver
>> >  - start/stop fix in replay mode with attached block devices
>> >  - record/replay of bh oneshot events, used in the block layer
>>
>> Can we come up with a reasonable smoke test to verify record/replay is
>> functional? AIUI we don't need a full OS but we do need a block device
>> to store the replay data. Could we use one of the simple system tests
>> like "memory" and run that through a record and replay cycle?
>
> That's would be great.
> I'm not familiar with testing framework and couldn't find "memory"
> test that you refer to.

The memory test code is in tests/tcg/multiarch/system and gets combined
with the boot code for whichever target can build it. For example on
aarch64 it is run like:

  timeout 15  $BLD/aarch64-softmmu/qemu-system-aarch64 -monitor none -display 
none -chardev file,path=memory.out,id=output  -M virt -cpu max -display none 
-semihosting-config enable=on,target=native,chardev=output -kernel memory

The test binaries will be in $BLD/tests/tcg/aarch64-softmmu and are
built when you run "make check-tcg" and have either the appropriate
cross compilers installed on your system or docker enabled and setup
(see docs/devel/testing.rst).

> Replay test must at least the following:
> 1. record some execution
> 2. replay that execution
>
> And there could be several endings:
> 1. record stalled
> 2. record crashed
> 3. replay stalled
> 4. replay crashed
> 5. all executions finished successfully

If you can provide an appropriate set of invocations I can plumb them
into the Makefiles for you.

>
> Pavel Dovgalyuk
>
>>
>> >
>> > ---
>> >
>> > Pavel Dovgaluk (6):
>> >   block: implement bdrv_snapshot_goto for blkreplay
>> >   replay: disable default snapshot for record/replay
>> >   replay: update docs for record/replay with block devices
>> >   replay: don't drain/flush bdrv queue while RR is working
>> >   replay: finish record/replay before closing the disks
>> >   replay: add BH oneshot event for block layer
>> >
>> >
>> >  block/blkreplay.c|8 
>> >  block/block-backend.c|9 ++---
>> >  block/io.c   |   32 ++--
>> >  block/iscsi.c|5 +++--
>> >  block/nfs.c  |6 --
>> >  block/null.c |4 +++-
>> >  block/nvme.c |6 --
>> >  block/rbd.c  |5 +++--
>> >  block/vxhs.c |5 +++--
>> >  cpus.c   |2 --
>> >  docs/replay.txt  |   12 +---
>> >  include/sysemu/replay.h  |4 
>> >  replay/replay-events.c   |   16 
>> >  replay/replay-internal.h |1 +
>> >  replay/replay.c  |2 ++
>> >  stubs/Makefile.objs  |1 +
>> >  stubs/replay-user.c  |9 +
>> >  vl.c |   11 +--
>> >  18 files changed, 115 insertions(+), 23 deletions(-)
>> >  create mode 100644 stubs/replay-user.c
>>
>>
>> --
>> Alex Bennée


--
Alex Bennée

Re: [Qemu-devel] [PATCH 03/13] hw: Move MC146818 device from hw/timer/ to hw/rtc/ subdirectory

[Philippe Mathieu-Daudé]

On 9/17/19 10:32 PM, Richard Henderson wrote:
> On 9/16/19 11:48 AM, Philippe Mathieu-Daudé wrote:
>>  include/hw/rtc/mc146818rtc.h | 38 
> 
> Same rebase failure as for patch 4?

This one is actually correct, it got moved from:

 include/hw/timer/mc146818rtc.h   | 14 

The difference is the addition of missing copyright/license.

Re: [Qemu-devel] [PATCH 03/13] hw: Move MC146818 device from hw/timer/ to hw/rtc/ subdirectory

[Philippe Mathieu-Daudé]

On 9/18/19 9:43 AM, Thomas Huth wrote:
> On 17/09/2019 12.03, Philippe Mathieu-Daudé wrote:
>> On 9/17/19 7:07 AM, Thomas Huth wrote:
>>> On 16/09/2019 17.48, Philippe Mathieu-Daudé wrote:
 The MC146818 is a Real Time Clock, not a timer.
 Move it under the hw/rtc/ subdirectory.

 Signed-off-by: Philippe Mathieu-Daudé 
>>> [...]
 diff --git a/include/hw/rtc/mc146818rtc.h b/include/hw/rtc/mc146818rtc.h
 new file mode 100644
 index 00..888e04f9ab
 --- /dev/null
 +++ b/include/hw/rtc/mc146818rtc.h
 @@ -0,0 +1,38 @@
 +/*
 + * QEMU MC146818 RTC emulation
 + *
 + * Copyright (c) 2003-2004 Fabrice Bellard
 + *
 + * Permission is hereby granted, free of charge, to any person obtaining 
 a copy
 + * of this software and associated documentation files (the "Software"), 
 to deal
 + * in the Software without restriction, including without limitation the 
 rights
 + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
 sell
 + * copies of the Software, and to permit persons to whom the Software is
 + * furnished to do so, subject to the following conditions:
 + *
 + * The above copyright notice and this permission notice shall be 
 included in
 + * all copies or substantial portions of the Software.
 + *
 + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
 EXPRESS OR
 + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
 MERCHANTABILITY,
 + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
 + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
 OTHER
 + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
 ARISING FROM,
 + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
 IN
 + * THE SOFTWARE.
 + */
>>>
>>> If you run "git blame" on the old header file, it does not seem like
>>> Fabrice wrote this header, so I'm not sure whether it makes sense to add
>>> his (c) statement here?
>>>
>>> Maybe rather use a one-line "SPDX-License-Identifier: GPL-2.0-or-later"
>>> here?
>>
>> It was first added by Fabrice here:
>>
>> $ git show 80cabfad163
> [...]
>> diff --git a/vl.h b/vl.h
>> index 35962d1985..026a5dee5a 100644
>> --- a/vl.h
>> +++ b/vl.h
>> +/* mc146818rtc.c */
>> +
>> +typedef struct RTCState {
>> +uint8_t cmos_data[128];
>> +uint8_t cmos_index;
>> +int irq;
>> +} RTCState;
>> +
>> +extern RTCState rtc_state;
>> +
>> +void rtc_init(int base, int irq);
>> +void rtc_timer(void);
> 
> Ok, fair. But vl.h had a slightly different copyright statement than
> vl.c, so I think you should rather use the one from vl.h.
> But IMHO you could at least drop the "THE SOFTWARE IS PROVIDED ..."
> paragraph and add a SPDX tag instead?

I find SPDX tags clearer too, but last time I wanted to use them Peter said:

  I think we should not do that until/unless
  somebody (probably a corporate somebody) steps forward
  to make the argument for "this is why we should have them,
  we as a contributor to the project think they are worthwhile
  and a useful feature for us, and we will make the effort to
  add them, review that they are correct, update checkpatch to
  insist on tags for new files, etc". In other words, "if it
  ain't broke, don't fix it"; nobody is yet complaining that
  our current setup is broken.

https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg04151.html

At this time we had less:

$ git grep SPDX v3.1.0 | egrep -v linux-headers | wc -l
21

Since we few entered, so we have a mix:

$ git grep SPDX origin/master | egrep -v linux-headers | wc -l
79

Re: [Qemu-devel] Problems with MIPS Malta SSH tests in make check-acceptance

[Philippe Mathieu-Daudé]

On 9/18/19 9:16 AM, David Gibson wrote:
> Hi,
> 
> I'm finding make check-acceptance is currently useless for me as a
> pre-pull test, because a bunch of the tests are not at all reliable.
> There are a bunch which I'm still investigating, but for now I'm
> looking at the MIPS Malta SSH tests.
> 
> There seem to be at least two problems here.  First, the test includes
> a download of a pretty big guest disk image.  This can easily exhaust
> the 2m30 timeout on its own.

Gerd raised this issue few months ago:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg615619.html

> Even without the timeout, it makes the test really slow, even on
> repeated runs.  Is there some way we can make the image download part
> of "building" the tests rather than actually running the testsuite, so
> that a) the test themselves go faster and b) we don't include the
> download in the test timeout - obviously the download speed is hugely
> dependent on factors that aren't really related to what we're testing
> here.
> 
> In the meantime, I tried hacking it by just increasing the timeout to
> 10m.  That got several of the tests working for me, but one still
> failed.  Specifically 'LinuxSSH.test_mips_malta32eb_kernel3_2_0' still
> timed out for me, but now after booting the guest, rather than during
> the image download.  Looking at the avocado log file I'm seeing a
> bunch of soft lockup messages from the guest console, AFAICT.  So it
> looks like we have a real bug here, which I suspect has been
> overlooked precisely because the download problems mean this test
> isn't reliable.
> 
> Any thoughts on how to improve the situation?

Maybe we should disable this test and run it manually...



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RESEND PATCH] tests/acceptance: Specify arch for QueryCPUModelExpansion

[Philippe Mathieu-Daudé]

On 9/18/19 9:06 AM, David Gibson wrote:
> At the moment this test runs on whatever the host arch is.  But it looks
> for 'unavailable-features' which is an x86 specific cpu property.  Tag it
> to always use qemu-system-x86_64.
> 
> Signed-off-by: David Gibson 
> Reviewed-by: Wainer dos Santos Moschetta 
> ---
>  tests/acceptance/cpu_queries.py | 3 +++
>  1 file changed, 3 insertions(+)
> 
> I sent this a while back, but it seems to have been forgotten.  As far
> as I can tell the current logic is Just Plain Wrong, on any host other
> than x86.
> 
> diff --git a/tests/acceptance/cpu_queries.py b/tests/acceptance/cpu_queries.py
> index e71edec39f..af47d2795a 100644
> --- a/tests/acceptance/cpu_queries.py
> +++ b/tests/acceptance/cpu_queries.py
> @@ -18,6 +18,9 @@ class QueryCPUModelExpansion(Test):
>  """
>  
>  def test(self):
> +"""
> +:avocado: tags=arch:x86_64
> +"""
>  self.vm.set_machine('none')
>  self.vm.add_args('-S')
>  self.vm.launch()
> 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH] iotests: Require Python 3.5 or later

[Philippe Mathieu-Daudé]

On 9/18/19 11:29 AM, Kevin Wolf wrote:
> Am 18.09.2019 um 11:20 hat Max Reitz geschrieben:
>> On 18.09.19 10:55, Kevin Wolf wrote:
>>> Running iotests is not required to build QEMU, so we can have stricter
>>> version requirements for Python here and can make use of new features
>>> and drop compatibility code earlier.
>>>
>>> This makes qemu-iotests skip all Python tests if a Python version before
>>> 3.5 is used for the build.

Good idea.

>>>
>>> Suggested-by: Eduardo Habkost 
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  tests/qemu-iotests/check | 14 +-
>>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>>> index 875399d79f..a68f414d6c 100755
>>> --- a/tests/qemu-iotests/check
>>> +++ b/tests/qemu-iotests/check
>>> @@ -633,6 +633,13 @@ then
>>>  export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
>>>  fi
>>>  
>>> +# Note that if the Python conditional here evaluates True we will exit
>>> +# with status 1 which is a shell 'false' value.
>>
>> I’d expect everything to exit with 1 if something does not work.  Thus,
>> I find the short script confusing (I think you do, too, or you wouldn’t
>> have written this comment).  Why not make it “sys.exit(0 if
>> sys.version_info >= (3, 5) else 1)”?

Good suggestion :)

> 
> I just copied it from configure, actually. :-)
> 
> But we can use your way, too. I don't really mind.

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH] qga: add command guest-get-devices for reporting VirtIO devices

[Tomáš Golembiovský]

On Mon, Sep 16, 2019 at 04:31:49PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Aug 29, 2019 at 8:06 PM Tomáš Golembiovský  
> wrote:
> >
> > Add command for reporting devices on Windows guest. The intent is not so
> > much to report the devices but more importantly the driver (and its
> > version) that is assigned to the device.
> >
> > Signed-off-by: Tomáš Golembiovský 
> > ---
> >  qga/commands-posix.c |  11 +++
> >  qga/commands-win32.c | 195 ++-
> >  qga/qapi-schema.json |  32 +++
> >  3 files changed, 237 insertions(+), 1 deletion(-)
> >
> > diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> > index dfc05f5b8a..9adf8bb520 100644
> > --- a/qga/commands-posix.c
> > +++ b/qga/commands-posix.c
> > @@ -2709,6 +2709,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
> >
> >  return 0;
> >  }
> > +
> >  #endif /* CONFIG_FSFREEZE */
> 
> extra line
> 
> 
> >
> >  #if !defined(CONFIG_FSTRIM)
> > @@ -2757,6 +2758,8 @@ GList *ga_command_blacklist_init(GList *blacklist)
> >  blacklist = g_list_append(blacklist, g_strdup("guest-fstrim"));
> >  #endif
> >
> > +blacklist = g_list_append(blacklist, g_strdup("guest-get-devices"));
> > +
> >  return blacklist;
> >  }
> >
> > @@ -2977,3 +2980,11 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> >
> >  return info;
> >  }
> > +
> > +GuestDeviceInfoList *qmp_guest_get_devices(Error **errp)
> > +{
> > +error_setg(errp, QERR_UNSUPPORTED);
> > +
> > +return NULL;
> > +}
> > +
> 
> extra EOF line
> 
> > diff --git a/qga/commands-win32.c b/qga/commands-win32.c
> > index 6b67f16faf..0bb93422c7 100644
> > --- a/qga/commands-win32.c
> > +++ b/qga/commands-win32.c
> > @@ -21,10 +21,11 @@
> >  #ifdef CONFIG_QGA_NTDDSCSI
> >  #include 
> >  #include 
> > +#endif
> >  #include 
> >  #include 
> >  #include 
> > -#endif
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -38,6 +39,36 @@
> >  #include "qemu/host-utils.h"
> >  #include "qemu/base64.h"
> >
> > +
> > +/* The following should be in devpkey.h, but it isn't */
> > +DEFINE_DEVPROPKEY(DEVPKEY_NAME, 0xb725f130, 0x47ef, 0x101a, 0xa5, 0xf1, 
> > 0x02,
> > +0x60, 0x8c, 0x9e, 0xeb, 0xac, 10);  /* DEVPROP_TYPE_STRING */
> > +DEFINE_DEVPROPKEY(DEVPKEY_Device_HardwareIds, 0xa45c254e, 0xdf1c, 0x4efd, 
> > 0x80,
> > +0x20, 0x67, 0xd1, 0x46, 0xa8, 0x50, 0xe0, 3);
> > +/* DEVPROP_TYPE_STRING_LIST */
> > +DEFINE_DEVPROPKEY(DEVPKEY_Device_DriverDate, 0xa8b865dd, 0x2e3d, 0x4094, 
> > 0xad,
> > +0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 2);  /* DEVPROP_TYPE_FILETIME 
> > */
> > +DEFINE_DEVPROPKEY(DEVPKEY_Device_DriverVersion, 0xa8b865dd, 0x2e3d, 0x4094,
> > +0xad, 0x97, 0xe5, 0x93, 0xa7, 0xc, 0x75, 0xd6, 3);
> 
> perhaps use a qemu prefix to avoid future clash?

Yes, this looks like the only option.

> 
> > +/* DEVPROP_TYPE_STRING */
> > +/* The following should be in sal.h, but it isn't */
> > +#ifndef _Out_writes_bytes_opt_
> > +#define _Out_writes_bytes_opt_(s)
> > +#endif
> 
> It got added in da215fcf5f001d1fdedf82e2f1407e8ff0b6d453
> ('include/sal: Update sal definitions').

Right, but that's still not available on CentOS. Anyway, I am dropping
all the _*_ keywords from the definition below in followup version.

> 
> #ifndef protects it, ok
> 
> > +/* The following shoud be in cfgmgr32.h, but it isn't */
> > +#ifndef CM_Get_DevNode_Property
> > +CMAPI CONFIGRET WINAPI CM_Get_DevNode_PropertyW(
> > +_In_  DEVINST   dnDevInst,
> > +_In_  CONST DEVPROPKEY* PropertyKey,
> > +_Out_ DEVPROPTYPE * PropertyType,
> > +_Out_writes_bytes_opt_(*PropertyBufferSize) PBYTE PropertyBuffer,
> > +_Inout_ PULONG  PropertyBufferSize,
> > +_In_  ULONG ulFlags
> > +);
> > +#define CM_Get_DevNode_Property  CM_Get_DevNode_PropertyW
> > +#endif
> > +
> 
> #ifndef should protect it from future clash, ok
> 
> Did you open bugs for mingw64 updates?

It was stuck on my TODO list for some time, but yeah I already posted a
patch for that. Thanks for reminding me.

> 
> > +
> >  #ifndef SHTDN_REASON_FLAG_PLANNED
> >  #define SHTDN_REASON_FLAG_PLANNED 0x8000
> >  #endif
> > @@ -2234,3 +2265,165 @@ GuestOSInfo *qmp_guest_get_osinfo(Error **errp)
> >
> >  return info;
> >  }
> > +
> > +/*
> > + * Safely get device property. Returned strings are using wide characters.
> > + * Caller is responsible for freeing the buffer.
> > + */
> > +static LPBYTE cm_get_property(DEVINST devInst, const DEVPROPKEY *propName,
> > +PDEVPROPTYPE propType)
> > +{
> > +CONFIGRET cr;
> > +LPBYTE buffer = NULL;
> > +ULONG bufferLen = 0;
> > +
> > +/* First query for needed space */
> > +cr = CM_Get_DevNode_Property(devInst, propName, propType,
> > +buffer, &bufferLen, 0);
> 
> I think qemu-ga win32 code generally prefers to be explicit about A()
> vs W(), call the W function explicitely, remove the generic define.
> 
> > +if ((cr != C

Re: [Qemu-devel] [PATCH v3 29/29] tests/tcg: target/s390x: Test MVO

[Alex Bennée]

David Hildenbrand  writes:

> On 18.09.19 11:47, Alex Bennée wrote:
>>
>> David Hildenbrand  writes:
>>
>>> Let's add the simple test based on the example from the PoP.
>>>
>>> Signed-off-by: David Hildenbrand 
>>> ---
>>>  tests/tcg/s390x/Makefile.target |  1 +
>>>  tests/tcg/s390x/mvo.c   | 25 +
>>>  2 files changed, 26 insertions(+)
>>>  create mode 100644 tests/tcg/s390x/mvo.c
>>>
>>> diff --git a/tests/tcg/s390x/Makefile.target 
>>> b/tests/tcg/s390x/Makefile.target
>>> index 151dc075aa..6a3bfa8b29 100644
>>> --- a/tests/tcg/s390x/Makefile.target
>>> +++ b/tests/tcg/s390x/Makefile.target
>>> @@ -6,3 +6,4 @@ TESTS+=ipm
>>>  TESTS+=exrl-trt
>>>  TESTS+=exrl-trtr
>>>  TESTS+=pack
>>> +TESTS+=mvo
>>> diff --git a/tests/tcg/s390x/mvo.c b/tests/tcg/s390x/mvo.c
>>> new file mode 100644
>>> index 00..5546fe2a97
>>> --- /dev/null
>>> +++ b/tests/tcg/s390x/mvo.c
>>> @@ -0,0 +1,25 @@
>>> +#include 
>>> +#include 
>>> +
>>> +int main(void)
>>> +{
>>> +uint8_t dest[6] = {0xff, 0x77, 0x88, 0x99, 0x0c, 0xff};
>>> +uint8_t src[5] = {0xee, 0x12, 0x34, 0x56, 0xee};
>>> +uint8_t expected[6] = {0xff, 0x01, 0x23, 0x45, 0x6c, 0xff};
>>> +int i;
>>> +
>>> +asm volatile (
>>> +"mvo 0(4,%[dest]),0(3,%[src])\n"
>>> +:
>>> +: [dest] "d" (dest + 1),
>>> +  [src] "d" (src + 1)
>>> +: "memory");
>>> +
>>> +for (i = 0; i < sizeof(expected); i++) {
>>> +if (dest[i] != expected[i]) {
>>> +fprintf(stderr, "bad data\n");
>>> +return 1;
>>> +}
>>> +}
>>> +return 0;
>>> +}
>>
>> Reviewed-by: Alex Bennée 
>>
>> but...
>>
>> can this test be expanded to check the page cross cases that caused you
>> so much trouble to track down?
>
> I might add a MVC test that tries to reproduce this. But with
> speculative page faults and things like that it might not be very easy
> to reproduce. However, I can give it a try.

I may not be fully understanding the correct behaviour but wouldn't the
test be:

  * map two page's worth of address space
  * mprot(!write) the top page
  * attempt mvc based copy to mmap region (say p0+(PAGE_SIZE>>1) to 
p1+(PAGE_SIZE>>1))
  * catch the fault
  * check the bottom page wasn't written to

or does the test need to be lower level and run in kernel mode in
softmmu and catch the appropriate low level exceptions?

>
> Thanks!
>
>>
>> --
>> Alex Bennée
>>


--
Alex Bennée

Re: [Qemu-devel] qemu patches

[Philippe Mathieu-Daudé]

Hi Ilias,

On 9/18/19 10:19 AM, Ilias wrote:
> Hi,
> 
> Your recent patch
> 
> https://github.com/qemu/qemu/commit/12e9493df9242a2051701e7eb64175d4e904acba#diff-d98f9b48cc332d226892f0db74a86b87
> 
> to the file
> 
> target/i386/whpx-all.c
> 
> 
> breaks compilation when WHPX is enabled.

How do you build QEMU? Which OS/compiler version, ./configure flags,
etc...? (looking at adding your case in our build setup).

Thanks,

Phil.

Re: [Qemu-devel] [for-4.2 PATCH 0/6] Block-related record/replay fixes

[Pavel Dovgalyuk]

> From: Alex Bennée [mailto:alex.ben...@linaro.org]
> Pavel Dovgalyuk  writes:
> 
> >> -Original Message-
> >> From: Alex Bennée [mailto:alex.ben...@linaro.org]
> >> Sent: Tuesday, September 17, 2019 10:02 PM
> >> To: Pavel Dovgalyuk
> >> Cc: qemu-devel@nongnu.org; kw...@redhat.com; peter.mayd...@linaro.org;
> >> crosthwaite.pe...@gmail.com; boost.li...@gmail.com; 
> >> artem.k.pisare...@gmail.com;
> >> quint...@redhat.com; ciro.santi...@gmail.com; jasow...@redhat.com; 
> >> m...@redhat.com;
> >> arm...@redhat.com; mre...@redhat.com; maria.klimushenk...@ispras.ru; 
> >> dovga...@ispras.ru;
> >> kra...@redhat.com; pavel.dovga...@ispras.ru; thomas.dull...@googlemail.com;
> >> pbonz...@redhat.com; dgilb...@redhat.com; r...@twiddle.net
> >> Subject: Re: [for-4.2 PATCH 0/6] Block-related record/replay fixes
> >>
> >>
> >> Pavel Dovgalyuk  writes:
> >>
> >> > The set of patches include the block-related updates
> >> > of the record/replay icount feature:
> >> >  - application of 'snapshot' option on the file layer instead of
> >> >the top one: command line and documentation fix
> >> >  - implementation of bdrv_snapshot_goto for blkreplay driver
> >> >  - start/stop fix in replay mode with attached block devices
> >> >  - record/replay of bh oneshot events, used in the block layer
> >>
> >> Can we come up with a reasonable smoke test to verify record/replay is
> >> functional? AIUI we don't need a full OS but we do need a block device
> >> to store the replay data. Could we use one of the simple system tests
> >> like "memory" and run that through a record and replay cycle?
> >
> > That's would be great.
> > I'm not familiar with testing framework and couldn't find "memory"
> > test that you refer to.
> 
> The memory test code is in tests/tcg/multiarch/system and gets combined
> with the boot code for whichever target can build it. For example on
> aarch64 it is run like:
> 
>   timeout 15  $BLD/aarch64-softmmu/qemu-system-aarch64 -monitor none -display 
> none -chardev
> file,path=memory.out,id=output  -M virt -cpu max -display none 
> -semihosting-config
> enable=on,target=native,chardev=output -kernel memory

Yes, rr testing could be that simple, but in full system emulation mode.
Simplest tests may be run even without the block devices:

qemu-system-aarch64 -monitor none -display none -chardev 
file,path=memory.out,id=output
-M virt -cpu max -kernel memory -net none -icount 
shift=5,rr=record,rrfile=record.bin

Better testing must include some block device like Linux boot image or 
something similar.

> The test binaries will be in $BLD/tests/tcg/aarch64-softmmu and are
> built when you run "make check-tcg" and have either the appropriate
> cross compilers installed on your system or docker enabled and setup
> (see docs/devel/testing.rst).
> 
> > Replay test must at least the following:
> > 1. record some execution
> > 2. replay that execution
> >
> > And there could be several endings:
> > 1. record stalled
> > 2. record crashed
> > 3. replay stalled
> > 4. replay crashed
> > 5. all executions finished successfully
> 
> If you can provide an appropriate set of invocations I can plumb them
> into the Makefiles for you.

That will be great, thank you.

Pavel Dovgalyuk

Re: [Qemu-devel] [PATCH v1 7/9] tests/tcg: add float_madds test to multiarch

[Alex Bennée]

Richard Henderson  writes:

> On 9/17/19 5:00 PM, Alex Bennée wrote:
 +add_f32_const(0x8p-152);
 +add_f32_const(0x8p-152);
 +add_f32_const(0x8p-152);
>>>
>>> Why are you adding 3 of the same?
>>
>> To replicate the 1841491 test case where the same number is used for
>> a/b/c
>
> Then we really ought to be more explicit about that.

I guess - I was hoping to piggy back on the general case. Maybe we
should just split that into a separate test case. We can at least re-use
the format and flag printing code and drop the behind the scenes magic
to "join" the constant table with extra test values.

> You're not doing full permutations on the sets of numbers, so does 
> incrementing
> a random index really test what you intended?

I did initially do the full permutation but for madds it all adds up
quite quickly. The shuffle we do here is a bit of a comprise, so it
shuffles the various nan forms before nans with -inf and -large and
finally a bunch of real numbers (subnormals in the middle). It gives a
reasonably broad coverage without going nuts.

>
 +#if defined(__arm__)
 +r = __builtin_fmaf(a, b, c);
 +#else
 +r = __builtin_fmaf(a, b, c);
 +#endif
>>>
>>> Eh?
>>
>> Ahh I was going to hardcode the arm madd instruction in as the builtin
>> wasn't expanding. I tried setting -march in the CFLAGS but that didn't
>> trigger it either on my buster arm-hf compiler. Any ideas how to get the
>> compiler to do the right thing?
>
> I think you want -mfpu=neon-vfpv4.

Ahh that worked. I would have hoped that v8 would be enough to spit out
aarch32 code which has to have VFPv4 for A profile right?

Anyway I'll use that for now.

>
>
> r~


--
Alex Bennée

Re: [Qemu-devel] [PATCH] tests/docker: fix typo for debian9-mxe

[Alex Bennée]

John Snow  writes:

> We spelled it debian-9-mxe, but the image is debian9-mxe.
>
> Signed-off-by: John Snow 

I'll merge this fix with the other typo fix while I'm at it. Thanks.

> ---
>  tests/docker/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 50a400b573..7eac1516f6 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -7,7 +7,7 @@ DOCKER_FILES_DIR := $(SRC_PATH)/tests/docker/dockerfiles
>  DOCKER_DEPRECATED_IMAGES := debian
>  # we don't run tests on intermediate images (used as base by another image)
>  DOCKER_PARTIAL_IMAGES := debian debian8 debian9 debian10 debian-sid
> -DEBIAN_PARTIAL_IMAGES += debian8-mxe debian-9-mxe debian-ports 
> debian-bootstrap
> +DEBIAN_PARTIAL_IMAGES += debian8-mxe debian9-mxe debian-ports 
> debian-bootstrap
>  DOCKER_IMAGES := $(filter-out $(DOCKER_DEPRECATED_IMAGES),$(sort $(notdir 
> $(basename $(wildcard $(DOCKER_FILES_DIR)/*.docker)
>  DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES))
>  # Use a global constant ccache directory to speed up repetitive builds


--
Alex Bennée

Re: [Qemu-devel] Problems with MIPS Malta SSH tests in make check-acceptance

[David Gibson]

On Wed, Sep 18, 2019 at 01:13:29PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/18/19 9:16 AM, David Gibson wrote:
> > Hi,
> > 
> > I'm finding make check-acceptance is currently useless for me as a
> > pre-pull test, because a bunch of the tests are not at all reliable.
> > There are a bunch which I'm still investigating, but for now I'm
> > looking at the MIPS Malta SSH tests.
> > 
> > There seem to be at least two problems here.  First, the test includes
> > a download of a pretty big guest disk image.  This can easily exhaust
> > the 2m30 timeout on its own.
> 
> Gerd raised this issue few months ago:
> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg615619.html

Ah, yes indeed.

> > Even without the timeout, it makes the test really slow, even on
> > repeated runs.  Is there some way we can make the image download part
> > of "building" the tests rather than actually running the testsuite, so
> > that a) the test themselves go faster and b) we don't include the
> > download in the test timeout - obviously the download speed is hugely
> > dependent on factors that aren't really related to what we're testing
> > here.
> > 
> > In the meantime, I tried hacking it by just increasing the timeout to
> > 10m.  That got several of the tests working for me, but one still
> > failed.  Specifically 'LinuxSSH.test_mips_malta32eb_kernel3_2_0' still
> > timed out for me, but now after booting the guest, rather than during
> > the image download.  Looking at the avocado log file I'm seeing a
> > bunch of soft lockup messages from the guest console, AFAICT.  So it
> > looks like we have a real bug here, which I suspect has been
> > overlooked precisely because the download problems mean this test
> > isn't reliable.
> > 
> > Any thoughts on how to improve the situation?
> 
> Maybe we should disable this test and run it manually...

Until we can fix it better, I really think we should.  A test this
unreliable verges on worse than useless.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 0/6] tests/acceptance: Add tests for the PReP/40p machine

[Philippe Mathieu-Daudé]

On 9/17/19 1:40 PM, David Gibson wrote:
> On Tue, Sep 17, 2019 at 12:19:52PM +1000, David Gibson wrote:
>> On Mon, Sep 16, 2019 at 11:56:06AM +0200, Philippe Mathieu-Daudé wrote:
>>> On 9/16/19 11:52 AM, Alex Bennée wrote:

 Philippe Mathieu-Daudé  writes:

> Hi David,
>
> On 9/16/19 2:42 AM, David Gibson wrote:
>> On Sun, Sep 15, 2019 at 11:19:34PM +0200, Philippe Mathieu-Daudé wrote:
>>> Quick tests worth to avoid regressions with the 40p machine.
>>> idea from the "Maintainers, please tell us how to boot your machines"
>>> thread:
>>> https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg04177.html
>>>
>>> v2: Split Travis job, added Hervé R-b tag
>>> v1: https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg05896.html
>>>
>>> Regards,
>>>
>>> Phil.
>>
>> I'm guessing you're expecting these to go in via the testing tree, in
>> which case
>>
>> Acked-by: David Gibson 
>
> Thanks, appreciated :)
>
>> Or do you want me to take them via the ppc tree?
>
> I think the 'testing tree' should focus on the CI/testing
> infrastructure, while each subsystem maintainers should care about the
> tests covering their subsystem (the testing tree maintainers might not
> have the required knowledge to be sure a test is correctly implemented).
>
> In this particular case I assume you don't have much knowledge of that
> PPC machine, which is a hobbyist one, but since you are the PPC
> maintainer, I'd rather see this going via your tree :)
>
> Alex/Cleber/Eduardo, any comment on this position?

 Once we have a .travis.yml I'm happy with it can go in via another tree
 no problem. See other thread
>>>
>>> Good :)
>>>
>>> David can take patches 1-5 (I tagged patch 6 as RFC but messed something
>>> with git-publish and lost it when I sent this series).
>>
>> Ok, I've taken patches 1-5 into my ppc-for-4.2 tree.
> 
> Hrm.  Judging by both the continued comments on this thread, and the
> fact it breaks the travis build, seems like this series needs a little
> more work.  I've pulled it out of ppc-for-4.2 again, and I'll wait for
> the next spin.

OK, sorry :|

[Qemu-devel] [PATCH] xen-block: treat XenbusStateUnknown the same as XenbusStateClosed

[Paul Durrant]

When a frontend gracefully disconnects from an offline backend, it will
set its own state to XenbusStateClosed. The code in xen-block.c correctly
deals with this and sets the backend into XenbusStateClosed. Unfortunately
it is possible for toolstack to actually delete the frontend area
before the state key has been read, leading to an apparent frontend state
of XenbusStateUnknown. This prevents the backend state from transitioning
to XenbusStateClosed and hence leaves it limbo.

This patch simply treats a frontend state of XenbusStateUnknown the same
as XenbusStateClosed, which will unblock the backend in these circumstances.

Reported-by: Mark Syms 
Signed-off-by: Paul Durrant 
---
Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Kevin Wolf 
Cc: Max Reitz 
---
 hw/block/xen-block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index f77343db60..879fc310a4 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -313,6 +313,7 @@ static void xen_block_frontend_changed(XenDevice *xendev,
 break;
 
 case XenbusStateClosed:
+case XenbusStateUnknown:
 xen_block_disconnect(xendev, &local_err);
 if (local_err) {
 error_propagate(errp, local_err);
-- 
2.20.1.2.gb21ebb671

[Qemu-devel] [PATCH] xen-bus: only set the xen device frontend state if it is missing

[Paul Durrant]

From: Mark Syms 

Some toolstack implementations will set the frontend xenstore
keys to Initialising which will then trigger the in guest PV
drivers to begin initialising and some implementations will
then set their state to Closing. If this has occurred then
device realize must not overwrite the frontend keys as then
the handshake will stall.

Signed-off-by: Mark Syms 

Also avoid creating the frontend area if it already exists.

Signed-off-by: Paul Durrant 
---
Cc: Stefano Stabellini 
Cc: Anthony Perard 
---
 hw/xen/xen-bus.c | 47 +++
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 55c157393d..c2ad22a42d 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -857,6 +857,13 @@ static void xen_device_frontend_changed(void *opaque)
 }
 }
 
+static bool xen_device_frontend_exists(XenDevice *xendev)
+{
+enum xenbus_state state;
+
+return (xen_device_frontend_scanf(xendev, "state", "%u", &state) == 1);
+}
+
 static void xen_device_frontend_create(XenDevice *xendev, Error **errp)
 {
 XenBus *xenbus = XEN_BUS(qdev_get_parent_bus(DEVICE(xendev)));
@@ -865,19 +872,25 @@ static void xen_device_frontend_create(XenDevice *xendev, 
Error **errp)
 
 xendev->frontend_path = xen_device_get_frontend_path(xendev);
 
-perms[0].id = xendev->frontend_id;
-perms[0].perms = XS_PERM_NONE;
-perms[1].id = xenbus->backend_id;
-perms[1].perms = XS_PERM_READ | XS_PERM_WRITE;
+/*
+ * The frontend area may have already been created by a legacy
+ * toolstack.
+ */
+if (!xen_device_frontend_exists(xendev)) {
+perms[0].id = xendev->frontend_id;
+perms[0].perms = XS_PERM_NONE;
+perms[1].id = xenbus->backend_id;
+perms[1].perms = XS_PERM_READ | XS_PERM_WRITE;
 
-g_assert(xenbus->xsh);
+g_assert(xenbus->xsh);
 
-xs_node_create(xenbus->xsh, XBT_NULL, xendev->frontend_path, perms,
-   ARRAY_SIZE(perms), &local_err);
-if (local_err) {
-error_propagate_prepend(errp, local_err,
-"failed to create frontend: ");
-return;
+xs_node_create(xenbus->xsh, XBT_NULL, xendev->frontend_path, perms,
+   ARRAY_SIZE(perms), &local_err);
+if (local_err) {
+error_propagate_prepend(errp, local_err,
+"failed to create frontend: ");
+return;
+}
 }
 
 xendev->frontend_state_watch =
@@ -1290,12 +1303,14 @@ static void xen_device_realize(DeviceState *dev, Error 
**errp)
 xen_device_backend_set_online(xendev, true);
 xen_device_backend_set_state(xendev, XenbusStateInitWait);
 
-xen_device_frontend_printf(xendev, "backend", "%s",
-   xendev->backend_path);
-xen_device_frontend_printf(xendev, "backend-id", "%u",
-   xenbus->backend_id);
+if (!xen_device_frontend_exists(xendev)) {
+xen_device_frontend_printf(xendev, "backend", "%s",
+   xendev->backend_path);
+xen_device_frontend_printf(xendev, "backend-id", "%u",
+   xenbus->backend_id);
 
-xen_device_frontend_set_state(xendev, XenbusStateInitialising, true);
+xen_device_frontend_set_state(xendev, XenbusStateInitialising, true);
+}
 
 xendev->exit.notify = xen_device_exit;
 qemu_add_exit_notifier(&xendev->exit);
-- 
2.20.1.2.gb21ebb671

Re: [Qemu-devel] WHPX build broken

[Philippe Mathieu-Daudé]

On 9/18/19 1:23 PM, Philippe Mathieu-Daudé wrote:
> Hi Ilias,
> 
> On 9/18/19 10:19 AM, Ilias wrote:
>> Hi,
>>
>> Your recent patch
>>
>> https://github.com/qemu/qemu/commit/12e9493df9242a2051701e7eb64175d4e904acba#diff-d98f9b48cc332d226892f0db74a86b87
>>
>> to the file
>>
>> target/i386/whpx-all.c
>> 
>>
>> breaks compilation when WHPX is enabled.
> 
> How do you build QEMU? Which OS/compiler version, ./configure flags,
> etc...? (looking at adding your case in our build setup).

OK I could reproduce wiht the qemu:fedora Docker image and this
Android/MinGW commit:
https://android.googlesource.com/platform/prebuilts/gcc/linux-x86/host/x86_64-w64-mingw32-4.8/+/1bde9c3b14f3a3b081ada6e32da9f2870671b46f

$ ./configure --python=/usr/bin/python3
--cross-prefix=x86_64-w64-mingw32-
--target-list=x86_64-softmmu,i386-softmmu --enable-hax --enable-whpx
--extra-cflags="-Iandroid" --disable-werror

$ make x86_64-softmmu/all
[...]
  CC  x86_64-softmmu/target/i386/whpx-all.o
target/i386/whpx-all.c: In function 'whpx_accel_init':
target/i386/whpx-all.c:1378:25: error: dereferencing pointer to
incomplete type 'MachineState' {aka 'struct MachineState'}
 whpx->mem_quota = ms->ram_size;
 ^~
make[1]: *** [rules.mak:69: target/i386/whpx-all.o] Error 1
  CC  x86_64-softmmu/trace/generated-helpers.o
make[1]: Target 'all' not remade because of errors.
make: *** [Makefile:471: x86_64-softmmu/all] Error 2

MachineState is indeed declared in "hw/boards.h".