[Qemu-devel] [Bug 1818937] Re: Crash with HV_ERROR on macOS host

2019-07-27 Thread Roman Bolshakov
My guess is that RFLAGS.ZF == 1 and one or a few of the checks on VMX controls 
have failed. So far I have verified the following checks (26-2 and 26-3 in 
Intel SDM Vol. 3C):
* Reserved bits in Pin-based VM execution controls are set according to 
associated capabilities MSR 
* Reserved bits in Primary Proc-based VM execution controls are set according 
to associated capabilities MSR 
* Reserved bits in Secondary Proc-based VM execution controls are set according 
to associated capabilities MSR 
* CR-3 target count is not greater than 4. (the count is 0)
* Use I/O bitmaps check is not applicable because "use I/O bitmaps" 
VM-execution control is 0.
* Reserved bits in VM-exit controls are set according to associated 
capabilities MSR 
* Reserved bits in VM-entry controls are set according to associated 
capabilities MSR 

However, the MSR-bitmap Address check might fail:
"If the “use MSR bitmaps” VM-execution control is 1, bits 11:0 of the 
MSR-bitmap address must be 0. The address should not set any bits beyond the 
processor’s physical-address width."

Bit 28 in Pin-based VM execution controls is set to 1 while the MSR
address has bits 5:1 set to 1 (0x3f). There's no way to disable the "use
MSR bitmaps" execution control so I'll try to make a patch that sets 4k-
page aligned MSR bitmap address.

Updated log lines show the VMX capabilities for the control fields and VMCS 
fields related to the failure:
qemu-system-x86_64: hv_vcpu_run failed
qemu-system-x86_64: exit reason:0x0030
qemu-system-x86_64: exit qualification: 0x0083
qemu-system-x86_64: instruction error:  0x0007
qemu-system-x86_64: VM-EXECUTION CONTROL FIELDS
qemu-system-x86_64: Pin-Based VM-Execution Controls
qemu-system-x86_64: pin based ctls: 0x003f
qemu-system-x86_64: pin based caps: 0x007f003f
qemu-system-x86_64: Processor-Based VM-Execution Controls
qemu-system-x86_64: pri proc based ctls:0x95206dfa
qemu-system-x86_64: pri proc based caps:0xfdf9fffe9500697a
qemu-system-x86_64: sec proc based ctls:0x00a3
qemu-system-x86_64: sec proc based caps:0x00011cef00a2
qemu-system-x86_64: CR3-Target Controls
qemu-system-x86_64: cr3 target count:   0x
qemu-system-x86_64: MSR-Bitmap Address: 0x003f
qemu-system-x86_64: VM-EXIT CONTROL FIELDS
qemu-system-x86_64: VM-Exit Controls
qemu-system-x86_64: vm exit ctls:   0x00236fff
qemu-system-x86_64: vm exit caps:   0x00636fff00236fff
qemu-system-x86_64: VM-ENTRY CONTROL FIELDS
qemu-system-x86_64: VM-Entry Controls
qemu-system-x86_64: vm entry ctls:  0x93ff
qemu-system-x86_64: vm entry caps:  0x93ff91ff
qemu-system-x86_64: Error: HV_ERROR

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1818937

Title:
  Crash with HV_ERROR on macOS host

Status in QEMU:
  New

Bug description:
  On macOS host running Windows 10 guest, qemu crashed with error
  message: Error: HV_ERROR.

  Host: macOS Mojave 10.14.3 (18D109) Late 2014 Mac mini presumably Core i5 
4278U.
  QEMU: git commit a3e3b0a7bd5de211a62cdf2d6c12b96d3c403560
  QEMU parameter: qemu-system-x86_64 -m 3000 -drive 
file=disk.img,if=virtio,discard=unmap -accel hvf -soundhw hda -smp 3

  thread list
  Process 56054 stopped
thread #1: tid = 0x2ffec8, 0x7fff48d0805a vImage`vLookupTable_Planar16 
+ 970, queue = 'com.apple.main-thread'
thread #2: tid = 0x2ffecc, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #3: tid = 0x2ffecd, 0x7fff79d715aa 
libsystem_kernel.dylib`__select + 10
thread #4: tid = 0x2ffece, 0x7fff79d71d9a 
libsystem_kernel.dylib`__sigwait + 10
  * thread #6: tid = 0x2ffed0, 0x7fff79d7023e 
libsystem_kernel.dylib`__pthread_kill + 10, stop reason = signal SIGABRT
thread #7: tid = 0x2ffed1, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #8: tid = 0x2ffed2, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #11: tid = 0x2fff34, 0x7fff79d6a17a 
libsystem_kernel.dylib`mach_msg_trap + 10, name = 'com.apple.NSEventThread'
thread #30: tid = 0x300c04, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #31: tid = 0x300c16, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #32: tid = 0x300c17, 0x
thread #33: tid = 0x300c93, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10

  
  Crashed thread:

  * thread #6, stop reason = signal SIGABRT
* frame #0: 0x7fff79d7023e libsystem_kernel.dylib`__pthread_kill + 10
  frame #1: 0x7fff79e26c1c libsystem_pthread.dylib`pthread_kill + 285
  frame #2: 0x7fff79cd91c9 libsystem_c.dylib`abort + 127
  frame #3: 0x00010baa476d 
qemu-system-x86_64`assert_hvf_ok(ret=) at hvf.c:106 [opt]
  fram

[Qemu-devel] [Bug 1818937] Re: Crash with HV_ERROR on macOS host

2019-07-30 Thread Roman Bolshakov
It's not possible to allocate MSR bitmap in userspace because it
requires a physical address to be stored in the VMCS field. However, the
bitmap page is already allocated inside kernel part of
Hypervisor.framework. The 4k bitmap region is aligned to page boundary.
It's worth to continue inspection of the checks (26.2 CHECKS ON VMX
CONTROLS AND HOST-STATE AREA).

The reason why MSR Bitmap Address has weird value is because it's not
necessarily the value of the VMCS field (albeit VMCS_CTRL_MSR_BITMAPS is
defined in hv_arch_vmx.h). HVF uses an internal lookup table that has a
limited set of VMCS fields exposed by Apple. The list is documented at
the reference page:
https://developer.apple.com/documentation/hypervisor/1469436-virtual_machine_control_structur

It's likely that 0x3f is a field from the VMCS lookup table. Given the
signature of hv_vmx_vcpu_read_vmcs, I would expect an error (e.g.
HV_BAD_ARGUMENT) to be returned instead of the silent failure. I have
submitted FB6858948 to Apple to correct the behaviour.

So, Apple doesn't provide an explicit access to MSR Bitmap Address field
but allows to control the bitmap via hv_vcpu_enable_native_msr.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1818937

Title:
  Crash with HV_ERROR on macOS host

Status in QEMU:
  New

Bug description:
  On macOS host running Windows 10 guest, qemu crashed with error
  message: Error: HV_ERROR.

  Host: macOS Mojave 10.14.3 (18D109) Late 2014 Mac mini presumably Core i5 
4278U.
  QEMU: git commit a3e3b0a7bd5de211a62cdf2d6c12b96d3c403560
  QEMU parameter: qemu-system-x86_64 -m 3000 -drive 
file=disk.img,if=virtio,discard=unmap -accel hvf -soundhw hda -smp 3

  thread list
  Process 56054 stopped
thread #1: tid = 0x2ffec8, 0x7fff48d0805a vImage`vLookupTable_Planar16 
+ 970, queue = 'com.apple.main-thread'
thread #2: tid = 0x2ffecc, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #3: tid = 0x2ffecd, 0x7fff79d715aa 
libsystem_kernel.dylib`__select + 10
thread #4: tid = 0x2ffece, 0x7fff79d71d9a 
libsystem_kernel.dylib`__sigwait + 10
  * thread #6: tid = 0x2ffed0, 0x7fff79d7023e 
libsystem_kernel.dylib`__pthread_kill + 10, stop reason = signal SIGABRT
thread #7: tid = 0x2ffed1, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #8: tid = 0x2ffed2, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #11: tid = 0x2fff34, 0x7fff79d6a17a 
libsystem_kernel.dylib`mach_msg_trap + 10, name = 'com.apple.NSEventThread'
thread #30: tid = 0x300c04, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #31: tid = 0x300c16, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #32: tid = 0x300c17, 0x
thread #33: tid = 0x300c93, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10

  
  Crashed thread:

  * thread #6, stop reason = signal SIGABRT
* frame #0: 0x7fff79d7023e libsystem_kernel.dylib`__pthread_kill + 10
  frame #1: 0x7fff79e26c1c libsystem_pthread.dylib`pthread_kill + 285
  frame #2: 0x7fff79cd91c9 libsystem_c.dylib`abort + 127
  frame #3: 0x00010baa476d 
qemu-system-x86_64`assert_hvf_ok(ret=) at hvf.c:106 [opt]
  frame #4: 0x00010baa4c8f 
qemu-system-x86_64`hvf_vcpu_exec(cpu=0x7f8e5283de00) at hvf.c:681 [opt]
  frame #5: 0x00010b988423 
qemu-system-x86_64`qemu_hvf_cpu_thread_fn(arg=0x7f8e5283de00) at 
cpus.c:1636 [opt]
  frame #6: 0x00010bd9dfce 
qemu-system-x86_64`qemu_thread_start(args=) at 
qemu-thread-posix.c:502 [opt]
  frame #7: 0x7fff79e24305 libsystem_pthread.dylib`_pthread_body + 126
  frame #8: 0x7fff79e2726f libsystem_pthread.dylib`_pthread_start + 70
  frame #9: 0x7fff79e23415 libsystem_pthread.dylib`thread_start + 13

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1818937/+subscriptions



[Qemu-devel] [Bug 1818937] Re: Crash with HV_ERROR on macOS host

2019-07-30 Thread Roman Bolshakov
During the inspection of Apple reference, I have noticed that Guest CR0 and CR0 
Guest/Host Mask has incorrect value. Apple defines that Guest CR0 is writable 
only if:
CR0.CD and CR0.NW are unset

But hvf accel code follows Intel SDM "Table 9-1. IA-32 and Intel 64
Processor States Following Power-up, Reset, or INIT" and sets CR0 value
to: 0x6010

Likewise, CR0 Guest/Host Mask is conditionally writable if:
CR0.CD and CR0.NW are set

I doubt if it's related to the HV_ERROR issue but I'll prepare a patch
to fix both fields (and likely set CR0 Read Shadow).

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1818937

Title:
  Crash with HV_ERROR on macOS host

Status in QEMU:
  New

Bug description:
  On macOS host running Windows 10 guest, qemu crashed with error
  message: Error: HV_ERROR.

  Host: macOS Mojave 10.14.3 (18D109) Late 2014 Mac mini presumably Core i5 
4278U.
  QEMU: git commit a3e3b0a7bd5de211a62cdf2d6c12b96d3c403560
  QEMU parameter: qemu-system-x86_64 -m 3000 -drive 
file=disk.img,if=virtio,discard=unmap -accel hvf -soundhw hda -smp 3

  thread list
  Process 56054 stopped
thread #1: tid = 0x2ffec8, 0x7fff48d0805a vImage`vLookupTable_Planar16 
+ 970, queue = 'com.apple.main-thread'
thread #2: tid = 0x2ffecc, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #3: tid = 0x2ffecd, 0x7fff79d715aa 
libsystem_kernel.dylib`__select + 10
thread #4: tid = 0x2ffece, 0x7fff79d71d9a 
libsystem_kernel.dylib`__sigwait + 10
  * thread #6: tid = 0x2ffed0, 0x7fff79d7023e 
libsystem_kernel.dylib`__pthread_kill + 10, stop reason = signal SIGABRT
thread #7: tid = 0x2ffed1, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #8: tid = 0x2ffed2, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #11: tid = 0x2fff34, 0x7fff79d6a17a 
libsystem_kernel.dylib`mach_msg_trap + 10, name = 'com.apple.NSEventThread'
thread #30: tid = 0x300c04, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #31: tid = 0x300c16, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #32: tid = 0x300c17, 0x
thread #33: tid = 0x300c93, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10

  
  Crashed thread:

  * thread #6, stop reason = signal SIGABRT
* frame #0: 0x7fff79d7023e libsystem_kernel.dylib`__pthread_kill + 10
  frame #1: 0x7fff79e26c1c libsystem_pthread.dylib`pthread_kill + 285
  frame #2: 0x7fff79cd91c9 libsystem_c.dylib`abort + 127
  frame #3: 0x00010baa476d 
qemu-system-x86_64`assert_hvf_ok(ret=) at hvf.c:106 [opt]
  frame #4: 0x00010baa4c8f 
qemu-system-x86_64`hvf_vcpu_exec(cpu=0x7f8e5283de00) at hvf.c:681 [opt]
  frame #5: 0x00010b988423 
qemu-system-x86_64`qemu_hvf_cpu_thread_fn(arg=0x7f8e5283de00) at 
cpus.c:1636 [opt]
  frame #6: 0x00010bd9dfce 
qemu-system-x86_64`qemu_thread_start(args=) at 
qemu-thread-posix.c:502 [opt]
  frame #7: 0x7fff79e24305 libsystem_pthread.dylib`_pthread_body + 126
  frame #8: 0x7fff79e2726f libsystem_pthread.dylib`_pthread_start + 70
  frame #9: 0x7fff79e23415 libsystem_pthread.dylib`thread_start + 13

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1818937/+subscriptions



Re: [PATCH v13 2/7] net/vmnet: add vmnet backends to qapi/net

2022-01-29 Thread Roman Bolshakov
On Tue, Jan 25, 2022 at 01:14:27PM +0900, Akihiko Odaki wrote:
> On Tue, Jan 25, 2022 at 8:00 AM Roman Bolshakov  wrote:
> >
> > On Mon, Jan 24, 2022 at 08:14:31PM +, Peter Maydell wrote:
> > > On Mon, 24 Jan 2022 at 17:49, Roman Bolshakov  wrote:
> > > > I'm not sure why blocks are Objective-C specific. All the data I have
> > > > shows the opposite [3][4][5]. They're just extensively used in Apple 
> > > > APIs.
> > >
> > > This is true, but for the purposes of our build machinery it is
> > > simpler to have three types of source files that it deals
> > > with (C, C++, ObjC) rather than four (C, C++, ObjC, C-that-uses-blocks).
> > > So unless there's a clear benefit from adding the extra category
> > > I think we should do the simple thing and keep these files named
> > > with a ".m" extension.
> > >
> >
> > Fine by me as long as majority finds it's simpler :) Perhaps it's just a
> > matter of personal preference.
> >
> > I've used to the fact that platform-specific code uses platform-specific
> > extensions or some sort of weird "GCC attributes". Therefore C with an
> > extension is easier to reason for me than Objective-C with ARC and other
> > kinds of implicit behaviour without an actual Objective-C code.
> >
> 
> Being technically pedantic, actually this vmnet implementation uses
> Objective-C and there is a file with .c which uses blocks.
> If a file is named .m, dispatch_retain(o) will be redefined as [o
> retain], and effectively makes it Objective-C code. Therefore, vmnet
> involves Objective-C as long as its files are named .m. It will be C
> with blocks if they are named .c.
> Speaking of use of blocks, actually audio/coreaudio.c involves blocks
> in header files; Core Audio has functions which accept blocks.
> 

Right, dispatch_retain()/dispatch_release() is just one example of the
implicit behaviour I'm talking about.

> I'm neutral about the decision.

> I think QEMU should avoid using Objective-C code except for
> interactions with Apple's APIs, and .c is superior in terms of that as
> it would prevent accidental introduction of Objective-C code.

That was exactly my point :)

> On the other hand, naming them .m will allow the
> introduction of Automatic Reference Counting to manage dispatch queue
> objects.

As of now ARC doesn't work automatically for .m files in QEMU. It
happens because QEMU doesn't enable it via -fobjc-arc.

If you try to enable it, Cocoa UI won't compile at all because of many
errors like this one and similar ones:

../ui/cocoa.m:1186:12: error: ARC forbids explicit message send of
'dealloc'
[super dealloc];
 ~ ^

> In fact, I have found a few memory leaks in vmnet in the last
> review and ui/cocoa.m has a suspicious construction of the object
> management (Particularly it has asynchronous dispatches wrapped with
> NSAutoreleasePool, which does not make sense).

> Introduction of Automatic Reference Counting would greatly help
> addressing those issues, but that would require significant rewriting
> of ui/cocoa.m.

Agreed.

Thanks,
Roman

P.S. I still think that given the mentioned facts and implicitness
introduced by Objective-C it would be more natural to have C code in
macOS-related device backends like vmnet and coreaudio unless
Objective-C is essential and required (like in UI code).

> Personally I'm concerned with ui/cocoa.m and do want to do that
> rewriting, but I'm being busy so it would not happen anytime soon.
> 
> Regards,
> Akihiko Odaki



Re: [PATCH v13 1/7] net/vmnet: add vmnet dependency and customizable option

2022-01-19 Thread Roman Bolshakov
On Thu, Jan 13, 2022 at 08:22:13PM +0300, Vladislav Yaroshchuk wrote:
> vmnet.framework dependency is added with 'vmnet' option
> to enable or disable it. Default value is 'auto'.
> 
> vmnet features to be used are available since macOS 11.0,

Hi Vladislav,

I'm not sure if the comment belongs here. Perhaps you mean that bridged
mode is available from 10.15:

VMNET_BRIDGED_MODE API_AVAILABLE(macos(10.15))  = 1002

This means vmnet.framework is supported on all macbooks starting from 2012.

With this fixed,
Tested-by: Roman Bolshakov 
Reviewed-by: Roman Bolshakov 

The other two modes - shared and host are supported on earlier versions
of macOS (from 10.10). But port forwarding is only available from macOS
10.15.

Theoretically it should possible to support the framework on the earlier
models from 2010 or 2007 on Yosemite up to High Sierra with less
features using MacPorts but I don't think it'd be reasonable to ask
that.

Thanks,
Roman

> corresponding probe is created into meson.build.
> 
> Signed-off-by: Vladislav Yaroshchuk 
> ---
>  meson.build   | 16 +++-
>  meson_options.txt |  2 ++
>  scripts/meson-buildoptions.sh |  3 +++
>  3 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index c1b1db1e28..285fb7bc41 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -496,6 +496,18 @@ if cocoa.found() and get_option('gtk').enabled()
>error('Cocoa and GTK+ cannot be enabled at the same time')
>  endif
>  
> +vmnet = dependency('appleframeworks', modules: 'vmnet', required: 
> get_option('vmnet'))
> +if vmnet.found() and not cc.has_header_symbol('vmnet/vmnet.h',
> +  'VMNET_BRIDGED_MODE',
> +  dependencies: vmnet)
> +  vmnet = not_found
> +  if get_option('vmnet').enabled()
> +error('vmnet.framework API is outdated')
> +  else
> +warning('vmnet.framework API is outdated, disabling')
> +  endif
> +endif
> +
>  seccomp = not_found
>  if not get_option('seccomp').auto() or have_system or have_tools
>seccomp = dependency('libseccomp', version: '>=2.3.0',
> @@ -1492,6 +1504,7 @@ config_host_data.set('CONFIG_SECCOMP', seccomp.found())
>  config_host_data.set('CONFIG_SNAPPY', snappy.found())
>  config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
>  config_host_data.set('CONFIG_VDE', vde.found())
> +config_host_data.set('CONFIG_VMNET', vmnet.found())
>  config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', 
> have_vhost_user_blk_server)
>  config_host_data.set('CONFIG_VNC', vnc.found())
>  config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
> @@ -3406,7 +3419,8 @@ summary(summary_info, bool_yn: true, section: 'Crypto')
>  # Libraries
>  summary_info = {}
>  if targetos == 'darwin'
> -  summary_info += {'Cocoa support':   cocoa}
> +  summary_info += {'Cocoa support':   cocoa}
> +  summary_info += {'vmnet.framework support': vmnet}
>  endif
>  summary_info += {'SDL support':   sdl}
>  summary_info += {'SDL image support': sdl_image}
> diff --git a/meson_options.txt b/meson_options.txt
> index 921967eddb..701e1381f9 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -151,6 +151,8 @@ option('netmap', type : 'feature', value : 'auto',
> description: 'netmap network backend support')
>  option('vde', type : 'feature', value : 'auto',
> description: 'vde network backend support')
> +option('vmnet', type : 'feature', value : 'auto',
> +   description: 'vmnet.framework network backend support')
>  option('virglrenderer', type : 'feature', value : 'auto',
> description: 'virgl rendering support')
>  option('vnc', type : 'feature', value : 'auto',
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 50bd7bed4d..cdcece4b05 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -84,6 +84,7 @@ meson_options_help() {
>printf "%s\n" '  u2f U2F emulation support'
>printf "%s\n" '  usb-redir   libusbredir support'
>printf "%s\n" '  vde vde network backend support'
> +  printf "%s

Re: [PATCH v13 2/7] net/vmnet: add vmnet backends to qapi/net

2022-01-20 Thread Roman Bolshakov
On Thu, Jan 13, 2022 at 08:22:14PM +0300, Vladislav Yaroshchuk wrote:
> Create separate netdevs for each vmnet operating mode:
> - vmnet-host
> - vmnet-shared
> - vmnet-bridged
> 

Sure I'm late to the party but what if we add only one backend - vmnet
with default mode set to shared and all parameters are added there?

The CLI would look more reasonable for the most typical use case:
 -netdev vmnet,id=if1 -device virtio-net,netdev=if1

That would remove duplication of options in QAPI schema (e.g. isolated
is available in all backends now, altough I'm not sure if it makes sense
for bridged mode):

 -netdev vmnet,id=if1,isolated=yes

start-address, end-address and subnet-mask are also used by both shared
and host modes.

Bridged netdev would lool like:

 -netdev vmnet,id=if1,mode=bridged,ifname=en1

Checksum offloading also seems to be available for all backends from
Monterey.

The approach might simplify integration of the changes to libvirt and
discovery of upcoming vmnet features via qapi.

Thanks,
Roman

> Signed-off-by: Vladislav Yaroshchuk 
> ---
>  net/clients.h   |  11 
>  net/meson.build |   7 +++
>  net/net.c   |  10 
>  net/vmnet-bridged.m |  25 +
>  net/vmnet-common.m  |  20 +++
>  net/vmnet-host.c|  24 
>  net/vmnet-shared.c  |  25 +
>  net/vmnet_int.h |  25 +
>  qapi/net.json   | 133 +++-
>  9 files changed, 278 insertions(+), 2 deletions(-)
>  create mode 100644 net/vmnet-bridged.m
>  create mode 100644 net/vmnet-common.m
>  create mode 100644 net/vmnet-host.c
>  create mode 100644 net/vmnet-shared.c
>  create mode 100644 net/vmnet_int.h
> 
> diff --git a/net/net.c b/net/net.c
> index f0d14dbfc1..1dbb64b935 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1021,6 +1021,11 @@ static int (* const 
> net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
>  #ifdef CONFIG_L2TPV3
>  [NET_CLIENT_DRIVER_L2TPV3]= net_init_l2tpv3,
>  #endif
> +#ifdef CONFIG_VMNET
> +[NET_CLIENT_DRIVER_VMNET_HOST] = net_init_vmnet_host,
> +[NET_CLIENT_DRIVER_VMNET_SHARED] = net_init_vmnet_shared,
> +[NET_CLIENT_DRIVER_VMNET_BRIDGED] = net_init_vmnet_bridged,
> +#endif /* CONFIG_VMNET */
>  };
>  
>  
> @@ -1106,6 +,11 @@ void show_netdevs(void)
>  #endif
>  #ifdef CONFIG_VHOST_VDPA
>  "vhost-vdpa",
> +#endif
> +#ifdef CONFIG_VMNET
> +"vmnet-host",
> +"vmnet-shared",
> +"vmnet-bridged",
>  #endif
>  };
>  
> diff --git a/net/vmnet-bridged.m b/net/vmnet-bridged.m
> new file mode 100644
> index 00..4e42a90391
> --- /dev/null
> +++ b/net/vmnet-bridged.m
> @@ -0,0 +1,25 @@
> +/*
> + * vmnet-bridged.m
> + *
> + * Copyright(c) 2021 Vladislav Yaroshchuk 
> + *
> + * 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 "qapi/qapi-types-net.h"
> +#include "vmnet_int.h"
> +#include "clients.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +
> +#include 
> +
> +int net_init_vmnet_bridged(const Netdev *netdev, const char *name,
> +   NetClientState *peer, Error **errp)
> +{
> +  error_setg(errp, "vmnet-bridged is not implemented yet");
> +  return -1;
> +}
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> new file mode 100644
> index 00..532d152840
> --- /dev/null
> +++ b/net/vmnet-common.m
> @@ -0,0 +1,20 @@
> +/*
> + * vmnet-common.m - network client wrapper for Apple vmnet.framework
> + *
> + * Copyright(c) 2021 Vladislav Yaroshchuk 
> + * Copyright(c) 2021 Phillip Tennen 
> + *
> + * 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 "qapi/qapi-types-net.h"
> +#include "vmnet_int.h"
> +#include "clients.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +
> +#include 
> +
> diff --git a/net/vmnet-host.c b/net/vmnet-host.c
> new file mode 100644
> index 00..4a5ef99dc7
> --- /dev/null
> +++ b/net/vmnet-host.c
> @@ -0,0 +1,24 @@
> +/*
> + * vmnet-host.c
> + *
> + * Copyright(c) 2021 Vladislav Yaroshchuk 
> + *
> + * 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 "qapi/qapi-types-net.h"
> +#include "vmnet_int.h"
> +#include "clients.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +
> +#include 
> +
> +int net_init_vmnet_host(const Netdev *netdev, const char *name,
> +NetClientState *peer, Error **errp) {
> +  error_setg(errp, "vmnet-host is not implemented yet");
> +  return -1;
> +}
> diff --git a/net/vmnet-shared.c b/net/vmnet-shared.c
> new file mode 100644
> index 00..f8c4a4f3b8
> --- /dev/null
> +++ b/net/vmnet-shared.c
> 

Re: [PATCH v13 2/7] net/vmnet: add vmnet backends to qapi/net

2022-01-24 Thread Roman Bolshakov
On Thu, Jan 13, 2022 at 08:22:14PM +0300, Vladislav Yaroshchuk wrote:
> Create separate netdevs for each vmnet operating mode:
> - vmnet-host
> - vmnet-shared
> - vmnet-bridged
> 
> Signed-off-by: Vladislav Yaroshchuk 
> ---
>  net/clients.h   |  11 
>  net/meson.build |   7 +++
>  net/net.c   |  10 
>  net/vmnet-bridged.m |  25 +
>  net/vmnet-common.m  |  20 +++

Hi Vladislav,

It seems the last two files should have .c extension rather than .m.

Unlike Cocoa UI code, the files do not contain Objective-C classes. They are
just C code with blocks (which is supported by compilers shipped with Xcode
SDK), e.g this program can be compiled without extra compiler flags:

$ cat block.c
int main() {
int (^x)(void) = ^{
return 0;
};

return x();
}
$ cc block.c && ./a.out
$

Regards,
Roman

>  net/vmnet-host.c|  24 
>  net/vmnet-shared.c  |  25 +
>  net/vmnet_int.h |  25 +
>  qapi/net.json   | 133 +++-
>  9 files changed, 278 insertions(+), 2 deletions(-)
>  create mode 100644 net/vmnet-bridged.m
>  create mode 100644 net/vmnet-common.m
>  create mode 100644 net/vmnet-host.c
>  create mode 100644 net/vmnet-shared.c
>  create mode 100644 net/vmnet_int.h
> 
> diff --git a/net/clients.h b/net/clients.h
> index 92f9b59aed..c9157789f2 100644
> --- a/net/clients.h
> +++ b/net/clients.h
> @@ -63,4 +63,15 @@ int net_init_vhost_user(const Netdev *netdev, const char 
> *name,
>  
>  int net_init_vhost_vdpa(const Netdev *netdev, const char *name,
>  NetClientState *peer, Error **errp);
> +#ifdef CONFIG_VMNET
> +int net_init_vmnet_host(const Netdev *netdev, const char *name,
> +  NetClientState *peer, Error **errp);
> +
> +int net_init_vmnet_shared(const Netdev *netdev, const char *name,
> +  NetClientState *peer, Error **errp);
> +
> +int net_init_vmnet_bridged(const Netdev *netdev, const char *name,
> +  NetClientState *peer, Error **errp);
> +#endif /* CONFIG_VMNET */
> +
>  #endif /* QEMU_NET_CLIENTS_H */
> diff --git a/net/meson.build b/net/meson.build
> index 847bc2ac85..00a88c4951 100644
> --- a/net/meson.build
> +++ b/net/meson.build
> @@ -42,4 +42,11 @@ softmmu_ss.add(when: 'CONFIG_POSIX', if_true: 
> files(tap_posix))
>  softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('tap-win32.c'))
>  softmmu_ss.add(when: 'CONFIG_VHOST_NET_VDPA', if_true: files('vhost-vdpa.c'))
>  
> +vmnet_files = files(
> +  'vmnet-common.m',
> +  'vmnet-bridged.m',
> +  'vmnet-host.c',
> +  'vmnet-shared.c'
> +)
> +softmmu_ss.add(when: vmnet, if_true: vmnet_files)
>  subdir('can')
> diff --git a/net/net.c b/net/net.c
> index f0d14dbfc1..1dbb64b935 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -1021,6 +1021,11 @@ static int (* const 
> net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
>  #ifdef CONFIG_L2TPV3
>  [NET_CLIENT_DRIVER_L2TPV3]= net_init_l2tpv3,
>  #endif
> +#ifdef CONFIG_VMNET
> +[NET_CLIENT_DRIVER_VMNET_HOST] = net_init_vmnet_host,
> +[NET_CLIENT_DRIVER_VMNET_SHARED] = net_init_vmnet_shared,
> +[NET_CLIENT_DRIVER_VMNET_BRIDGED] = net_init_vmnet_bridged,
> +#endif /* CONFIG_VMNET */
>  };
>  
>  
> @@ -1106,6 +,11 @@ void show_netdevs(void)
>  #endif
>  #ifdef CONFIG_VHOST_VDPA
>  "vhost-vdpa",
> +#endif
> +#ifdef CONFIG_VMNET
> +"vmnet-host",
> +"vmnet-shared",
> +"vmnet-bridged",
>  #endif
>  };
>  
> diff --git a/net/vmnet-bridged.m b/net/vmnet-bridged.m
> new file mode 100644
> index 00..4e42a90391
> --- /dev/null
> +++ b/net/vmnet-bridged.m
> @@ -0,0 +1,25 @@
> +/*
> + * vmnet-bridged.m
> + *
> + * Copyright(c) 2021 Vladislav Yaroshchuk 
> + *
> + * 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 "qapi/qapi-types-net.h"
> +#include "vmnet_int.h"
> +#include "clients.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +
> +#include 
> +
> +int net_init_vmnet_bridged(const Netdev *netdev, const char *name,
> +   NetClientState *peer, Error **errp)
> +{
> +  error_setg(errp, "vmnet-bridged is not implemented yet");
> +  return -1;
> +}
> diff --git a/net/vmnet-common.m b/net/vmnet-common.m
> new file mode 100644
> index 00..532d152840
> --- /dev/null
> +++ b/net/vmnet-common.m
> @@ -0,0 +1,20 @@
> +/*
> + * vmnet-common.m - network client wrapper for Apple vmnet.framework
> + *
> + * Copyright(c) 2021 Vladislav Yaroshchuk 
> + * Copyright(c) 2021 Phillip Tennen 
> + *
> + * 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 "qapi/qapi-types-net.h"
> +#include "vmnet_int.h"
> +#include "clients.h

Re: [PATCH v13 2/7] net/vmnet: add vmnet backends to qapi/net

2022-01-24 Thread Roman Bolshakov
On Mon, Jan 24, 2022 at 12:27:40PM +0100, Christian Schoenebeck wrote:
> On Montag, 24. Januar 2022 10:56:00 CET Roman Bolshakov wrote:
> > On Thu, Jan 13, 2022 at 08:22:14PM +0300, Vladislav Yaroshchuk wrote:
> > >  net/vmnet-bridged.m |  25 +
> > >  net/vmnet-common.m  |  20 +++
> > 
> > It seems the last two files should have .c extension rather than .m.
> 
> I would not do that. Mind cross-compilers, please.
> 

Hi Christian,

Cross-compilers for Apple platforms can be constructed using à la carte
approach where toolchain comes from the source, SDK from Apple and a
port of cctools from GitHub (mind all library dependencies of QEMU).
That's quite an effort!

I very much doubt this is a relevant and typical case for QEMU on macOS.
And if cross-compiler is constructed properly it'll pass required flags
that enable blocks and will link block runtime in its default build
recipe like all cross-compilers do for the platform of interest.

Gladly, there's osxcross [1] and crossbuild image with Darwin support [2].
They can deal with blocks just fine:

  # CROSS_TRIPLE=i386-apple-darwin
  $ cc block.c && file a.out
  a.out: Mach-O i386 executable, 
flags:

  # CROSS_TRIPLE=x86_64-apple-darwin
  $ cc block.c && file a.out
  $ file a.out
  a.out: Mach-O 64-bit x86_64 executable, flags:

> > Unlike Cocoa UI code, the files do not contain Objective-C classes. They are
> > just C code with blocks (which is supported by compilers shipped with Xcode
> > SDK), e.g this program can be compiled without extra compiler flags:
> > 
> > $ cat block.c
> > int main() {
> > int (^x)(void) = ^{
> > return 0;
> > };
> > 
> > return x();
> > }
> > $ cc block.c && ./a.out
> > $
> > 
> 
> Such blocks are still Objective-C language specific, they are not C and 
> therefore won't work with GCC.
> 

I'm not sure why blocks are Objective-C specific. All the data I have
shows the opposite [3][4][5]. They're just extensively used in Apple APIs.

> $ gcc block.c
> 
> block.c: In function ‘main’:
> block.c:2:14: error: expected identifier or ‘(’ before ‘^’ token
>  int (^x)(void) = ^{
>   ^
> block.c:6:16: warning: implicit declaration of function ‘x’ [-Wimplicit-
> function-declaration]
>  return x();
> ^

You might do this on Linux and it'll work:

$ clang -g -fblocks -lBlocksRuntime block.c && ./a.out

However, vmnet code won't be compiled on non-Apple platforms because the
compilation happens only if vmnet is available which happens only if
appleframeworks dependency is available, that is not available on
non-OSX hosts [6]:

  "These dependencies can never be found for non-OSX hosts."

1. https://github.com/tpoechtrager/osxcross
2. https://github.com/multiarch/crossbuild
3. http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1370.pdf
4. https://clang.llvm.org/docs/BlockLanguageSpec.html
5. https://clang.llvm.org/docs/Block-ABI-Apple.html
6. https://mesonbuild.com/Dependencies.html#appleframeworks

Regards,
Roman



Re: [PATCH v13 2/7] net/vmnet: add vmnet backends to qapi/net

2022-01-24 Thread Roman Bolshakov
On Mon, Jan 24, 2022 at 08:14:31PM +, Peter Maydell wrote:
> On Mon, 24 Jan 2022 at 17:49, Roman Bolshakov  wrote:
> > I'm not sure why blocks are Objective-C specific. All the data I have
> > shows the opposite [3][4][5]. They're just extensively used in Apple APIs.
> 
> This is true, but for the purposes of our build machinery it is
> simpler to have three types of source files that it deals
> with (C, C++, ObjC) rather than four (C, C++, ObjC, C-that-uses-blocks).
> So unless there's a clear benefit from adding the extra category
> I think we should do the simple thing and keep these files named
> with a ".m" extension.
> 

Fine by me as long as majority finds it's simpler :) Perhaps it's just a
matter of personal preference.

I've used to the fact that platform-specific code uses platform-specific
extensions or some sort of weird "GCC attributes". Therefore C with an
extension is easier to reason for me than Objective-C with ARC and other
kinds of implicit behaviour without an actual Objective-C code.

Thanks,
Roman



Re: [RFC PATCH v3 0/7] host: Support macOS 12

2022-01-10 Thread Roman Bolshakov
On Mon, Jan 10, 2022 at 02:09:54PM +0100, Philippe Mathieu-Daudé wrote:
> Few patches to be able to build QEMU on macOS 12 (Monterey).
> 
> This basically consists of adapting deprecated APIs. I am not
> sure about these APIs, so tagging as RFC.
> 
> I couldn't succeed to adapt the Cocoa code.
> 
> CI job added to avoid bitrotting.
> 
> Since v2:
> - Addressed Akihiko Odaki comments:
>   . use __is_identifier(),
>   . remove cocoa setAllowedFileTypes()
> - Addressed Daniel Berrangé comment:
>   . rebased on testing/next, update libvirt-ci/lcitool
> 
> Based on Alex's testing/next

Hi Philippe,

Could you please share URI to the remote?
I want to apply the series on it.

Thanks,
Roman

> Based-on: <20220110124638.610145-1-f4...@amsat.org>
> 
> Philippe Mathieu-Daudé (7):
>   configure: Allow passing extra Objective C compiler flags
>   ui/cocoa: Remove allowedFileTypes restriction in SavePanel
>   hvf: Make hvf_get_segments() / hvf_put_segments() local
>   hvf: Remove deprecated hv_vcpu_flush() calls
>   audio/coreaudio: Remove a deprecation warning on macOS 12
>   block/file-posix: Remove a deprecation warning on macOS 12
>   gitlab-ci: Support macOS 12 via cirrus-run
> 
>  configure |  8 
>  meson.build   |  5 +
>  target/i386/hvf/vmx.h |  2 --
>  target/i386/hvf/x86hvf.h  |  2 --
>  audio/coreaudio.c | 16 ++--
>  block/file-posix.c| 13 +
>  target/i386/hvf/x86_task.c|  1 -
>  target/i386/hvf/x86hvf.c  |  6 ++
>  .gitlab-ci.d/cirrus.yml   | 15 +++
>  .gitlab-ci.d/cirrus/macos-12.vars | 16 
>  tests/lcitool/libvirt-ci  |  2 +-
>  tests/lcitool/refresh |  1 +
>  ui/cocoa.m|  6 --
>  13 files changed, 67 insertions(+), 26 deletions(-)
>  create mode 100644 .gitlab-ci.d/cirrus/macos-12.vars
> 
> -- 
> 2.33.1
> 



Re: [RFC PATCH v3 2/7] ui/cocoa: Remove allowedFileTypes restriction in SavePanel

2022-01-10 Thread Roman Bolshakov
On Mon, Jan 10, 2022 at 02:09:56PM +0100, Philippe Mathieu-Daudé wrote:
> setAllowedFileTypes is deprecated in macOS 12.
> 
> Per Akihiko Odaki [*]:
> 
>   An image file, which is being chosen by the panel, can be a
>   raw file and have a variety of file extensions and many are not
>   covered by the provided list (e.g. "udf"). Other platforms like
>   GTK can provide an option to open a file with an extension not
>   listed, but Cocoa can't.
>
> It forces the user to rename the file
>   to give an extension in the list. Moreover, Cocoa does not tell
>   which extensions are in the list so the user needs to read the
>   source code, which is pretty bad.
> 
> Since this code is harming the usability rather than improving it,
> simply remove the [NSSavePanel allowedFileTypes:] call, fixing:
> 

Yes, it is an issue for raw images with extensions outside of the
specified list or for images without any extension.

Reviewed-by: Roman Bolshakov 
Tested-by: Roman Bolshakov 

Regards,
-Roman

>   [2789/6622] Compiling Objective-C object libcommon.fa.p/ui_cocoa.m.o
>   ui/cocoa.m:1411:16: error: 'setAllowedFileTypes:' is deprecated: first 
> deprecated in macOS 12.0 - Use -allowedContentTypes instead 
> [-Werror,-Wdeprecated-declarations]
>   [openPanel setAllowedFileTypes: supportedImageFileTypes];
>  ^
>   
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49:
>  note: property 'allowedFileTypes' is declared deprecated here
>   @property (nullable, copy) NSArray *allowedFileTypes 
> API_DEPRECATED("Use -allowedContentTypes instead", macos(10.3,12.0));
>   ^
>   
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/AppKit.framework/Headers/NSSavePanel.h:215:49:
>  note: 'setAllowedFileTypes:' has been explicitly marked deprecated here
>   FAILED: libcommon.fa.p/ui_cocoa.m.o
> 
> [*] 
> https://lore.kernel.org/qemu-devel/4dde2e66-63cb-4390-9538-c032310db...@gmail.com/
> 
> Suggested-by: Akihiko Odaki 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  ui/cocoa.m | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 69745c483b4..dec22968815 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -100,7 +100,6 @@ static void cocoa_switch(DisplayChangeListener *dcl,
>  static char **gArgv;
>  static bool stretch_video;
>  static NSTextField *pauseLabel;
> -static NSArray * supportedImageFileTypes;
>  
>  static QemuSemaphore display_init_sem;
>  static QemuSemaphore app_started_sem;
> @@ -1162,10 +1161,6 @@ - (id) init
>  [pauseLabel setTextColor: [NSColor blackColor]];
>  [pauseLabel sizeToFit];
>  
> -// set the supported image file types that can be opened
> -supportedImageFileTypes = [NSArray arrayWithObjects: @"img", @"iso", 
> @"dmg",
> - @"qcow", @"qcow2", @"cloop", @"vmdk", 
> @"cdr",
> -  @"toast", nil];
>  [self make_about_window];
>  }
>  return self;
> @@ -1408,7 +1403,6 @@ - (void)changeDeviceMedia:(id)sender
>  openPanel = [NSOpenPanel openPanel];
>  [openPanel setCanChooseFiles: YES];
>  [openPanel setAllowsMultipleSelection: NO];
> -[openPanel setAllowedFileTypes: supportedImageFileTypes];
>  if([openPanel runModal] == NSModalResponseOK) {
>  NSString * file = [[[openPanel URLs] objectAtIndex: 0] path];
>  if(file == nil) {
> -- 
> 2.33.1
> 



Re: [RFC PATCH v3 3/7] hvf: Make hvf_get_segments() / hvf_put_segments() local

2022-01-11 Thread Roman Bolshakov
On Mon, Jan 10, 2022 at 02:09:57PM +0100, Philippe Mathieu-Daudé wrote:
> Both hvf_get_segments/hvf_put_segments() functions are only
> used within x86hvf.c: do not declare them as public API.
> 

Reviewed-by: Roman Bolshakov 
Tested-by: Roman Bolshakov 

Thanks,
Roman

> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  target/i386/hvf/x86hvf.h | 2 --
>  target/i386/hvf/x86hvf.c | 4 ++--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/target/i386/hvf/x86hvf.h b/target/i386/hvf/x86hvf.h
> index 99ed8d608dd..db6003d6bda 100644
> --- a/target/i386/hvf/x86hvf.h
> +++ b/target/i386/hvf/x86hvf.h
> @@ -26,11 +26,9 @@ void hvf_set_segment(struct CPUState *cpu, struct 
> vmx_segment *vmx_seg,
>   SegmentCache *qseg, bool is_tr);
>  void hvf_get_segment(SegmentCache *qseg, struct vmx_segment *vmx_seg);
>  void hvf_put_xsave(CPUState *cpu_state);
> -void hvf_put_segments(CPUState *cpu_state);
>  void hvf_put_msrs(CPUState *cpu_state);
>  void hvf_get_xsave(CPUState *cpu_state);
>  void hvf_get_msrs(CPUState *cpu_state);
>  void vmx_clear_int_window_exiting(CPUState *cpu);
> -void hvf_get_segments(CPUState *cpu_state);
>  void vmx_update_tpr(CPUState *cpu);
>  #endif
> diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
> index 05ec1bddc4e..907f09f1b43 100644
> --- a/target/i386/hvf/x86hvf.c
> +++ b/target/i386/hvf/x86hvf.c
> @@ -83,7 +83,7 @@ void hvf_put_xsave(CPUState *cpu_state)
>  }
>  }
>  
> -void hvf_put_segments(CPUState *cpu_state)
> +static void hvf_put_segments(CPUState *cpu_state)
>  {
>  CPUX86State *env = &X86_CPU(cpu_state)->env;
>  struct vmx_segment seg;
> @@ -166,7 +166,7 @@ void hvf_get_xsave(CPUState *cpu_state)
>  x86_cpu_xrstor_all_areas(X86_CPU(cpu_state), xsave, xsave_len);
>  }
>  
> -void hvf_get_segments(CPUState *cpu_state)
> +static void hvf_get_segments(CPUState *cpu_state)
>  {
>  CPUX86State *env = &X86_CPU(cpu_state)->env;
>  
> -- 
> 2.33.1



Re: [RFC PATCH v3 1/7] configure: Allow passing extra Objective C compiler flags

2022-01-11 Thread Roman Bolshakov
On Mon, Jan 10, 2022 at 02:09:55PM +0100, Philippe Mathieu-Daudé wrote:
> We can pass C/CPP/LD flags via CFLAGS/CXXFLAGS/LDFLAGS environment
> variables, or via configure --extra-cflags / --extra-cxxflags /
> --extra-ldflags options. Provide similar behavior for Objective C:
> use existing flags from $OBJCFLAGS, or passed via --extra-objcflags.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  configure   | 8 
>  meson.build | 5 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/configure b/configure
> index 0c57a063c66..adb42d8beb1 100755
> --- a/configure
> +++ b/configure
> @@ -288,6 +288,7 @@ done
>  
>  EXTRA_CFLAGS=""
>  EXTRA_CXXFLAGS=""
> +EXTRA_OBJCFLAGS=""
>  EXTRA_LDFLAGS=""
>  
>  xen_ctrl_version="$default_feature"
> @@ -400,9 +401,12 @@ for opt do
>--extra-cflags=*)
>  EXTRA_CFLAGS="$EXTRA_CFLAGS $optarg"
>  EXTRA_CXXFLAGS="$EXTRA_CXXFLAGS $optarg"
> +EXTRA_OBJCFLAGS="$EXTRA_OBJCFLAGS $optarg"
>  ;;
>--extra-cxxflags=*) EXTRA_CXXFLAGS="$EXTRA_CXXFLAGS $optarg"
>;;
> +  --extra-objcflags=*) EXTRA_OBJCFLAGS="$EXTRA_OBJCFLAGS $optarg"
> +  ;;
>--extra-ldflags=*) EXTRA_LDFLAGS="$EXTRA_LDFLAGS $optarg"
>;;
>--enable-debug-info) debug_info="yes"
> @@ -781,6 +785,8 @@ for opt do
>;;
>--extra-cxxflags=*)
>;;
> +  --extra-objcflags=*)
> +  ;;
>--extra-ldflags=*)
>;;
>--enable-debug-info)
> @@ -1318,6 +1324,7 @@ Advanced options (experts only):
>--objcc=OBJCCuse Objective-C compiler OBJCC [$objcc]
>--extra-cflags=CFLAGSappend extra C compiler flags CFLAGS
>--extra-cxxflags=CXXFLAGS append extra C++ compiler flags CXXFLAGS
> +  --extra-objcflags=OBJCFLAGS append extra Objective C compiler flags 
> OBJCFLAGS
>--extra-ldflags=LDFLAGS  append extra linker flags LDFLAGS
>--cross-cc-ARCH=CC   use compiler when building ARCH guest test cases
>--cross-cc-flags-ARCH=   use compiler flags when building ARCH guest tests
> @@ -3843,6 +3850,7 @@ if test "$skip_meson" = no; then
>echo "[built-in options]" >> $cross
>echo "c_args = [$(meson_quote $CFLAGS $EXTRA_CFLAGS)]" >> $cross
>echo "cpp_args = [$(meson_quote $CXXFLAGS $EXTRA_CXXFLAGS)]" >> $cross
> +  test -n "$objcc" && echo "objc_args = [$(meson_quote $OBJCFLAGS 
> $EXTRA_OBJCFLAGS)]" >> $cross
>echo "c_link_args = [$(meson_quote $CFLAGS $LDFLAGS $EXTRA_CFLAGS 
> $EXTRA_LDFLAGS)]" >> $cross
>echo "cpp_link_args = [$(meson_quote $CXXFLAGS $LDFLAGS $EXTRA_CXXFLAGS 
> $EXTRA_LDFLAGS)]" >> $cross
>echo "[binaries]" >> $cross
> diff --git a/meson.build b/meson.build
> index 0e52f54b100..a21305d62c1 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3286,6 +3286,11 @@
> + ['-O' + 
> get_option('optimization')]
> + (get_option('debug') ? 
> ['-g'] : []))}
>  endif
> +if targetos == 'darwin'
> +  summary_info += {'OBJCFLAGS':   ' '.join(get_option('objc_args')
> +   + ['-O' + 
> get_option('optimization')]
> +   + (get_option('debug') ? 
> ['-g'] : []))}

Hi Philippe,

You need to add something like below to actually use the flags in build:

add_global_arguments(config_host['QEMU_OBJCFLAGS'].split(),
 native: false, language: 'objc')

Regards,
Roman

> +endif
>  link_args = get_option(link_language + '_link_args')
>  if link_args.length() > 0
>summary_info += {'LDFLAGS': ' '.join(link_args)}
> -- 
> 2.33.1



Re: [RFC PATCH v3 4/7] hvf: Remove deprecated hv_vcpu_flush() calls

2022-01-11 Thread Roman Bolshakov
On Mon, Jan 10, 2022 at 02:09:58PM +0100, Philippe Mathieu-Daudé wrote:
> When building on macOS 12, we get:
> 
>   In file included from ../target/i386/hvf/hvf.c:59:
>   ../target/i386/hvf/vmx.h:174:5: error: 'hv_vcpu_flush' is deprecated: first 
> deprecated in macOS 11.0 - This API has no effect and always returns 
> HV_UNSUPPORTED [-Werror,-Wdeprecated-declarations]
>   hv_vcpu_flush(vcpu);
>   ^

This seems to be true even for older macOS (e.g. Catalina).

>   
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/Hypervisor.framework/Headers/hv.h:364:20:
>  note: 'hv_vcpu_flush' has been explicitly marked deprecated here
>   extern hv_return_t hv_vcpu_flush(hv_vcpuid_t vcpu)
>  ^
> 
> Since this call "has no effect", simply remove it ¯\_(ツ)_/¯
> 
> Not very useful deprecation doc:
> https://developer.apple.com/documentation/hypervisor/1441386-hv_vcpu_flush
> 

Reviewed-by: Roman Bolshakov 
Tested-by: Roman Bolshakov 

Thanks,
Roman

> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  target/i386/hvf/vmx.h  | 2 --
>  target/i386/hvf/x86_task.c | 1 -
>  target/i386/hvf/x86hvf.c   | 2 --
>  3 files changed, 5 deletions(-)
> 
> diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
> index 6df87116f62..094fb9b9dc9 100644
> --- a/target/i386/hvf/vmx.h
> +++ b/target/i386/hvf/vmx.h
> @@ -159,7 +159,6 @@ static inline void macvm_set_cr0(hv_vcpuid_t vcpu, 
> uint64_t cr0)
>  wvmcs(vcpu, VMCS_GUEST_CR0, cr0 | CR0_NE | CR0_ET);
>  
>  hv_vcpu_invalidate_tlb(vcpu);
> -hv_vcpu_flush(vcpu);
>  }
>  
>  static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t cr4)
> @@ -171,7 +170,6 @@ static inline void macvm_set_cr4(hv_vcpuid_t vcpu, 
> uint64_t cr4)
>  wvmcs(vcpu, VMCS_CR4_MASK, CR4_VMXE);
>  
>  hv_vcpu_invalidate_tlb(vcpu);
> -hv_vcpu_flush(vcpu);
>  }
>  
>  static inline void macvm_set_rip(CPUState *cpu, uint64_t rip)
> diff --git a/target/i386/hvf/x86_task.c b/target/i386/hvf/x86_task.c
> index 422156128b7..c8dc3d48fa8 100644
> --- a/target/i386/hvf/x86_task.c
> +++ b/target/i386/hvf/x86_task.c
> @@ -181,5 +181,4 @@ void vmx_handle_task_switch(CPUState *cpu, 
> x68_segment_selector tss_sel, int rea
>  store_regs(cpu);
>  
>  hv_vcpu_invalidate_tlb(cpu->hvf->fd);
> -hv_vcpu_flush(cpu->hvf->fd);
>  }
> diff --git a/target/i386/hvf/x86hvf.c b/target/i386/hvf/x86hvf.c
> index 907f09f1b43..bec9fc58146 100644
> --- a/target/i386/hvf/x86hvf.c
> +++ b/target/i386/hvf/x86hvf.c
> @@ -125,8 +125,6 @@ static void hvf_put_segments(CPUState *cpu_state)
>  
>  hvf_set_segment(cpu_state, &seg, &env->ldt, false);
>  vmx_write_segment_descriptor(cpu_state, &seg, R_LDTR);
> -
> -hv_vcpu_flush(cpu_state->hvf->fd);
>  }
>  
>  void hvf_put_msrs(CPUState *cpu_state)
> -- 
> 2.33.1



Re: [RFC PATCH v3 5/7] audio/coreaudio: Remove a deprecation warning on macOS 12

2022-01-11 Thread Roman Bolshakov
On Mon, Jan 10, 2022 at 02:09:59PM +0100, Philippe Mathieu-Daudé wrote:
> When building on macOS 12 we get:
> 
>   audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is 
> deprecated: first deprecated in macOS 12.0 [-Werror,-Wdeprecated-declarations]
>   kAudioObjectPropertyElementMaster
>   ^
>   kAudioObjectPropertyElementMain
>   
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5:
>  note: 'kAudioObjectPropertyElementMaster' has been explicitly marked 
> deprecated here
>   kAudioObjectPropertyElementMaster 
> API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", 
> macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) = 
> kAudioObjectPropertyElementMain
>   ^
> 
> Replace by kAudioObjectPropertyElementMain, redefining it to
> kAudioObjectPropertyElementMaster if not available, using
> Clang __is_identifier() feature (coreaudio is restricted to
> macOS).
> 

As of now it breaks the build on Catalina/10.15:

FAILED: libcommon.fa.p/audio_coreaudio.c.o
cc <...>
../audio/coreaudio.c:54:5: error: use of undeclared identifier 
'kAudioObjectPropertyElementMain'; did you mean 
'kAudioObjectPropertyElementName'?
kAudioObjectPropertyElementMain
^~~
kAudioObjectPropertyElementName

But __is_identifier itself works... Weird.

> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Checkpatch:
> 
>  WARNING: architecture specific defines should be avoided
>  #10: FILE: audio/coreaudio.c:47:
>  +#if !__is_identifier(kAudioObjectPropertyElementMain) /* macOS >= 12.0 */
> 
> Should we define __is_identifier() to 0 for GCC on macOS?

Clang documentation has this snippet:

#ifdef __is_identifier  // Compatibility with non-clang compilers.
  #if __is_identifier(__wchar_t)
typedef wchar_t __wchar_t;
  #endif
#endif

We can also add ifdef around just to be nice to GCC if it ever comes back on
macOS :)

Regards,
Roman

> ---
>  audio/coreaudio.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> index d8a21d3e507..73cbfd479ac 100644
> --- a/audio/coreaudio.c
> +++ b/audio/coreaudio.c
> @@ -44,10 +44,14 @@ typedef struct coreaudioVoiceOut {
>  bool enabled;
>  } coreaudioVoiceOut;
>  
> +#if !__is_identifier(kAudioObjectPropertyElementMain) /* macOS >= 12.0 */
> +#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
> +#endif
> +
>  static const AudioObjectPropertyAddress voice_addr = {
>  kAudioHardwarePropertyDefaultOutputDevice,
>  kAudioObjectPropertyScopeGlobal,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
>  };
>  
>  static OSStatus coreaudio_get_voice(AudioDeviceID *id)
> @@ -69,7 +73,7 @@ static OSStatus coreaudio_get_framesizerange(AudioDeviceID 
> id,
>  AudioObjectPropertyAddress addr = {
>  kAudioDevicePropertyBufferFrameSizeRange,
>  kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
>  };
>  
>  return AudioObjectGetPropertyData(id,
> @@ -86,7 +90,7 @@ static OSStatus coreaudio_get_framesize(AudioDeviceID id, 
> UInt32 *framesize)
>  AudioObjectPropertyAddress addr = {
>  kAudioDevicePropertyBufferFrameSize,
>  kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
>  };
>  
>  return AudioObjectGetPropertyData(id,
> @@ -103,7 +107,7 @@ static OSStatus coreaudio_set_framesize(AudioDeviceID id, 
> UInt32 *framesize)
>  AudioObjectPropertyAddress addr = {
>  kAudioDevicePropertyBufferFrameSize,
>  kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
>  };
>  
>  return AudioObjectSetPropertyData(id,
> @@ -121,7 +125,7 @@ static OSStatus coreaudio_set_streamformat(AudioDeviceID 
> id,
>  AudioObjectPropertyAddress addr = {
>  kAudioDevicePropertyStreamFormat,
>  kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
>  };
>  
>  return AudioObjectSetPropertyData(id,
> @@ -138,7 +142,7 @@ static OSStatus coreaudio_get_isrunning(AudioDeviceID id, 
> UInt32 *result)
>  AudioObjectPropertyAddress addr = {
>  kAudioDevicePropertyDeviceIsRunning,
>  kAudioDevicePropertyScopeOutput,
> -kAudioObjectPropertyElementMaster
> +kAudioObjectPropertyElementMain
>  };
>  
>  return AudioObjectGetPropertyData(id,
> -- 
> 2.33.1



Re: [RFC PATCH v3 5/7] audio/coreaudio: Remove a deprecation warning on macOS 12

2022-01-11 Thread Roman Bolshakov
On Mon, Jan 10, 2022 at 02:09:59PM +0100, Philippe Mathieu-Daudé wrote:
> When building on macOS 12 we get:
> 
>   audio/coreaudio.c:50:5: error: 'kAudioObjectPropertyElementMaster' is 
> deprecated: first deprecated in macOS 12.0 [-Werror,-Wdeprecated-declarations]
>   kAudioObjectPropertyElementMaster
>   ^
>   kAudioObjectPropertyElementMain
>   
> /Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/System/Library/Frameworks/CoreAudio.framework/Headers/AudioHardwareBase.h:208:5:
>  note: 'kAudioObjectPropertyElementMaster' has been explicitly marked 
> deprecated here
>   kAudioObjectPropertyElementMaster 
> API_DEPRECATED_WITH_REPLACEMENT("kAudioObjectPropertyElementMain", 
> macos(10.0, 12.0), ios(2.0, 15.0), watchos(1.0, 8.0), tvos(9.0, 15.0)) = 
> kAudioObjectPropertyElementMain
>   ^
> 
> Replace by kAudioObjectPropertyElementMain, redefining it to
> kAudioObjectPropertyElementMaster if not available, using
> Clang __is_identifier() feature (coreaudio is restricted to
> macOS).
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Checkpatch:
> 
>  WARNING: architecture specific defines should be avoided
>  #10: FILE: audio/coreaudio.c:47:
>  +#if !__is_identifier(kAudioObjectPropertyElementMain) /* macOS >= 12.0 */
> 
> Should we define __is_identifier() to 0 for GCC on macOS?
> ---
>  audio/coreaudio.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/audio/coreaudio.c b/audio/coreaudio.c
> index d8a21d3e507..73cbfd479ac 100644
> --- a/audio/coreaudio.c
> +++ b/audio/coreaudio.c
> @@ -44,10 +44,14 @@ typedef struct coreaudioVoiceOut {
>  bool enabled;
>  } coreaudioVoiceOut;
>  
> +#if !__is_identifier(kAudioObjectPropertyElementMain) /* macOS >= 12.0 */
> +#define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
> +#endif

Christian and Akihiko are right you need to replace it with macOS version
wrappers:

diff --git a/audio/coreaudio.c b/audio/coreaudio.c
index 73cbfd479a..7367a2ffd4 100644
--- a/audio/coreaudio.c
+++ b/audio/coreaudio.c
@@ -44,7 +44,8 @@ typedef struct coreaudioVoiceOut {
 bool enabled;
 } coreaudioVoiceOut;

-#if !__is_identifier(kAudioObjectPropertyElementMain) /* macOS >= 12.0 */
+#if !defined(MAC_OS_VERSION_12_0) || \
+(MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
 #define kAudioObjectPropertyElementMain kAudioObjectPropertyElementMaster
 #endif


And in the patch 6 you'd do likewise:

diff --git a/block/file-posix.c b/block/file-posix.c
index 1d0512026c..c0038629a1 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3325,7 +3325,8 @@ BlockDriver bdrv_file = {
 static kern_return_t GetBSDPath(io_iterator_t mediaIterator, char *bsdPath,
 CFIndex maxPathSize, int flags);

-#if !__is_identifier(IOMainPort) /* macOS >= 12.0 */
+#if !defined(MAC_OS_VERSION_12_0) || \
+(MAC_OS_X_VERSION_MAX_ALLOWED < MAC_OS_VERSION_12_0)
 #define IOMainPort IOMasterPort
 #endif

This way it the build would work also on older macOS.


Two more issues are left:

1. Linker has corrupted paths to clang directory (happens on all macOS 
versions).

Monterey:

[732/737] Linking target qemu-system-mips-unsigned
ld: warning: directory not found for option 
'-Lns/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/13.0.0'
[733/737] Linking target qemu-system-mips64-unsigned
ld: warning: directory not found for option 
'-Lns/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/clang/13.0.0'
[737/737] Generating qemu-system-mips64 with a custom command

Catalina:

ld: warning: directory not found for option 
'-Lveloper/CommandLineTools/usr/lib/clang/11.0.0'
[102/105] Linking target qemu-system-or1k-unsigned
ld: warning: directory not found for option 
'-Lveloper/CommandLineTools/usr/lib/clang/11.0.0'
[104/105] Linking target qemu-system-ppc-unsigned
ld: warning: directory not found for option 
'-Lveloper/CommandLineTools/usr/lib/clang/11.0.0'
[105/105] Generating qemu-system-ppc with a custom command

2. QEMU tests show FENV_ACCESS warning on Monterey:


[409/771] Compiling C object 
tests/fp/libtestfloat.a.p/berkeley-testfloat-3_source_test_az_f128_rx.c.o
../tests/fp/berkeley-testfloat-3/source/test_az_f128_rx.c:49:14: warning: 
'#pragma FENV_ACCESS' is not supported on this target - ignored 
[-Wignored-pragmas]
#pragma STDC FENV_ACCESS ON
 ^
1 warning generated.
[410/771] Compiling C object 
tests/fp/libtestfloat.a.p/berkeley-testfloat-3_source_test_abcz_f128.c.o
../tests/fp/berkeley-testfloat-3/source/test_abcz_f128.c:48:14: warning: 
'#pragma FENV_ACCESS' is not supported on this target - ignored 
[-Wignored-pragmas]
#pragma STDC FENV_ACCESS ON
 ^
1 warning generated.

Regards,
Roman

> +
>  static const AudioObjectPropertyAddress voice_addr = {
>  kAudioHardwarePropertyDefaultOutputDevice,
>  kAudioObjectPropertyScopeGlobal,
> -kAudioObjectPropertyEle

Re: [PULL 00/13] Net patches

2022-01-11 Thread Roman Bolshakov
On Wed, Jan 12, 2022 at 01:39:28PM +0800, Jason Wang wrote:
> 
> 在 2022/1/12 上午6:02, Vladislav Yaroshchuk 写道:
> > 
> > 
> > вт, 11 янв. 2022 г., 5:10 AM Jason Wang :
> > 
> > On Tue, Jan 11, 2022 at 12:49 AM Peter Maydell
> >  wrote:
> > >
> > > On Mon, 10 Jan 2022 at 03:40, Jason Wang 
> > wrote:
> > > >
> > > > The following changes since commit
> > df722e33d5da26ea8604500ca8f509245a0ea524:
> > > >
> > > >   Merge tag 'bsd-user-arm-pull-request' of
> > gitlab.com:bsdimp/qemu into staging (2022-01-08 09:37:59 -0800)
> > > >
> > > > are available in the git repository at:
> > > >
> > > > https://github.com/jasowang/qemu.git tags/net-pull-request
> > > >
> > > > for you to fetch changes up to
> > 5136cc6d3b8b74f4fa572f0874656947a401330e:
> > > >
> > > >   net/vmnet: update MAINTAINERS list (2022-01-10 11:30:55 +0800)
> > > >
> > > > 
> > > >
> > > > 
> > >
> > > Fails to build on OSX Catalina:
> > >
> > > ../../net/vmnet-common.m:165:10: error: use of undeclared identifier
> > > 'VMNET_SHARING_SERVICE_BUSY'
> > >     case VMNET_SHARING_SERVICE_BUSY:
> > >          ^
> > >
> > > This constant only got added in macOS 11.0. I guess that technically
> > > our supported-platforms policy only requires us to support 11
> > (Big Sur)
> > > and 12 (Monterey) at this point, but it would be nice to still
> > be able
> > > to build on Catalina (10.15).
> > 
> > Yes, it was only supported by the vmnet framework starting from
> > Catalyst according to
> > https://developer.apple.com/documentation/vmnet?language=objc.
> > 
> > 
> > Yes, there are some symbols from macOS >= 11.0 new backend
> > uses, not only this one, ex. vmnet_enable_isolation_key:
> > https://developer.apple.com/documentation/vmnet/vmnet_enable_isolation_key
> > 
> > >
> > > (Personally I would like Catalina still to work at least for a
> > little
> > > while, because my x86 Mac is old enough that it is not supported by
> > > Big Sur. I'll have to dump it once Apple stops doing security
> > support
> > > for Catalina, but they haven't done that quite yet.)
> > 
> > 
> > Sure, broken builds on old macOSes are bad. For this case I think
> > it's enough to disable vmnet for macOS < 11.0 with a probe while
> > configure build step. Especially given that Apple supports ~three
> > latest macOS versions, support for Catalina is expected to end
> > in 2022, when QEMU releases 7.0.
> 
> 
> That should be fine.
> 

I agree with Peter on this,

There's a lot of hardware running with Catalina. I think it's useful to
support it a little longer.

Regards,
Roman

> 
> > 
> > If this workaround is not suitable and it's required to support vmnet
> > in Catalina 10.15 with a subset of available features, it can be done.
> > But I'll be ready to handle this in approximately two-three weeks only.
> > 
> > Sure, Vladislav please fix this and send a new version.
> > 
> > 
> > Quick fix as described above is available in v10:
> > https://patchew.org/QEMU/20220111211422.21789-1-yaroshchuk2...@gmail.com/
> 
> 
> Have you got chance to test that for macOS < 11.0?
> 
> Thanks
> 
> 
> > Thanks
> > 
> > >
> > > -- PMM
> > >
> > 
> > 
> > 
> > 
> > -- 
> > Best Regards,
> > 
> > Vladislav Yaroshchuk
> 
> 



Re: [PATCH v10 0/7] Add vmnet.framework based network backend

2022-01-11 Thread Roman Bolshakov
On Wed, Jan 12, 2022 at 12:14:15AM +0300, Vladislav Yaroshchuk wrote:
> macOS provides networking API for VMs called 'vmnet.framework':
> https://developer.apple.com/documentation/vmnet
> 
> We can provide its support as the new QEMU network backends which
> represent three different vmnet.framework interface usage modes:
> 
>   * `vmnet-shared`:
> allows the guest to communicate with other guests in shared mode and
> also with external network (Internet) via NAT. Has (macOS-provided)
> DHCP server; subnet mask and IP range can be configured;
> 
>   * `vmnet-host`:
> allows the guest to communicate with other guests in host mode.
> By default has enabled DHCP as `vmnet-shared`, but providing
> network unique id (uuid) can make `vmnet-host` interfaces isolated
> from each other and also disables DHCP.
> 
>   * `vmnet-bridged`:
> bridges the guest with a physical network interface.
> 
> This backends cannot work on macOS Catalina 10.15 cause we use
> vmnet.framework API provided only with macOS 11 and newer. Seems
> that it is not a problem, because QEMU guarantees to work on two most
> recent versions of macOS which now are Big Sur (11) and Monterey (12).
> 
> Also, we have one inconvenient restriction: vmnet.framework interfaces
> can create only privileged user:
> `$ sudo qemu-system-x86_64 -nic vmnet-shared`
> 
> Attempt of `vmnet-*` netdev creation being unprivileged user fails with
> vmnet's 'general failure'.
> 
> This happens because vmnet.framework requires `com.apple.vm.networking`
> entitlement which is: "restricted to developers of virtualization software.
> To request this entitlement, contact your Apple representative." as Apple
> documentation says:
> https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_vm_networking
> 
> One more note: we still have quite useful but not supported
> 'vmnet.framework' features as creating port forwarding rules, IPv6
> NAT prefix specifying and so on.
> 
> Nevertheless, new backends work fine and tested within `qemu-system-x86-64`
> on macOS Bir Sur 11.5.2 host with such nic models:
>   * e1000-82545em
>   * virtio-net-pci
>   * vmxnet3
> 
> The guests were:
>   * macOS 10.15.7
>   * Ubuntu Bionic (server cloudimg)
> 
> 
> This series partially reuses patches by Phillip Tennen:
> https://patchew.org/QEMU/20210218134947.1860-1-phillip.en...@gmail.com/
> So I included them signed-off line into one of the commit messages and
> also here.
> 
> v1 -> v2:
>  Since v1 minor typos were fixed, patches rebased onto latest master,
>  redundant changes removed (small commits squashed)
> v2 -> v3:
>  - QAPI style fixes
>  - Typos fixes in comments
>  - `#include`'s updated to be in sync with recent master
> v3 -> v4:
>  - Support vmnet interfaces isolation feature
>  - Support vmnet-host network uuid setting feature
>  - Refactored sources a bit
> v4 -> v5:
>  - Missed 6.2 boat, now 7.0 candidate
>  - Fix qapi netdev descriptions and styles
>(@subnetmask -> @subnet-mask)
>  - Support vmnet-shared IPv6 prefix setting feature
> v5 -> v6
>  - provide detailed commit messages for commits of
>many changes
>  - rename properties @dhcpstart and @dhcpend to
>@start-address and @end-address
>  - improve qapi documentation about isolation
>features (@isolated, @net-uuid)
> v6 -> v7:
>  - update MAINTAINERS list
> v7 -> v8
>  - QAPI code style fixes
> v8 -> v9
>  - Fix building on Linux: add missing qapi
>`'if': 'CONFIG_VMNET'` statement to Netdev union
> v9 -> v10
>  - Disable vmnet feature for macOS < 11.0: add
>vmnet.framework API probe into meson.build.
>This fixes QEMU building on macOS < 11.0:
>https://patchew.org/QEMU/20220110034000.20221-1-jasow...@redhat.com/
> 

Hi Vladislav,

What symbols are missing on Catalina except VMNET_SHARING_BUSY?

It'd be great to get the feature working there.

Thanks,
Roman

> Vladislav Yaroshchuk (7):
>   net/vmnet: add vmnet dependency and customizable option
>   net/vmnet: add vmnet backends to qapi/net
>   net/vmnet: implement shared mode (vmnet-shared)
>   net/vmnet: implement host mode (vmnet-host)
>   net/vmnet: implement bridged mode (vmnet-bridged)
>   net/vmnet: update qemu-options.hx
>   net/vmnet: update MAINTAINERS list
> 
>  MAINTAINERS   |   5 +
>  meson.build   |  16 +-
>  meson_options.txt |   2 +
>  net/clients.h |  11 ++
>  net/meson.build   |   7 +
>  net/net.c |  10 ++
>  net/vmnet-bridged.m   | 111 
>  net/vmnet-common.m| 330 ++
>  net/vmnet-host.c  | 105 +++
>  net/vmnet-shared.c|  92 ++
>  net/vmnet_int.h   |  48 +
>  qapi/net.json | 132 +-
>  qemu-options.hx   |  25 +++
>  scripts/meson-buildoptions.sh |   3 +
>  14 files changed, 894 insertions(+), 3 deletions(-)
>  create mode 100644

Re: [PATCH v10 0/7] Add vmnet.framework based network backend

2022-01-12 Thread Roman Bolshakov
On Wed, Jan 12, 2022 at 10:50:04AM +0300, Roman Bolshakov wrote:
> On Wed, Jan 12, 2022 at 12:14:15AM +0300, Vladislav Yaroshchuk wrote:
> > macOS provides networking API for VMs called 'vmnet.framework':
> > https://developer.apple.com/documentation/vmnet
> > 
> > We can provide its support as the new QEMU network backends which
> > represent three different vmnet.framework interface usage modes:
> > 
> >   * `vmnet-shared`:
> > allows the guest to communicate with other guests in shared mode and
> > also with external network (Internet) via NAT. Has (macOS-provided)
> > DHCP server; subnet mask and IP range can be configured;
> > 
> >   * `vmnet-host`:
> > allows the guest to communicate with other guests in host mode.
> > By default has enabled DHCP as `vmnet-shared`, but providing
> > network unique id (uuid) can make `vmnet-host` interfaces isolated
> > from each other and also disables DHCP.
> > 
> >   * `vmnet-bridged`:
> > bridges the guest with a physical network interface.
> > 
> > This backends cannot work on macOS Catalina 10.15 cause we use
> > vmnet.framework API provided only with macOS 11 and newer. Seems
> > that it is not a problem, because QEMU guarantees to work on two most
> > recent versions of macOS which now are Big Sur (11) and Monterey (12).
> > 
> > Also, we have one inconvenient restriction: vmnet.framework interfaces
> > can create only privileged user:
> > `$ sudo qemu-system-x86_64 -nic vmnet-shared`
> > 
> > Attempt of `vmnet-*` netdev creation being unprivileged user fails with
> > vmnet's 'general failure'.
> > 
> > This happens because vmnet.framework requires `com.apple.vm.networking`
> > entitlement which is: "restricted to developers of virtualization software.
> > To request this entitlement, contact your Apple representative." as Apple
> > documentation says:
> > https://developer.apple.com/documentation/bundleresources/entitlements/com_apple_vm_networking
> > 
> > One more note: we still have quite useful but not supported
> > 'vmnet.framework' features as creating port forwarding rules, IPv6
> > NAT prefix specifying and so on.
> > 
> > Nevertheless, new backends work fine and tested within `qemu-system-x86-64`
> > on macOS Bir Sur 11.5.2 host with such nic models:
> >   * e1000-82545em
> >   * virtio-net-pci
> >   * vmxnet3
> > 
> > The guests were:
> >   * macOS 10.15.7
> >   * Ubuntu Bionic (server cloudimg)
> > 
> > 
> > This series partially reuses patches by Phillip Tennen:
> > https://patchew.org/QEMU/20210218134947.1860-1-phillip.en...@gmail.com/
> > So I included them signed-off line into one of the commit messages and
> > also here.
> > 
> > v1 -> v2:
> >  Since v1 minor typos were fixed, patches rebased onto latest master,
> >  redundant changes removed (small commits squashed)
> > v2 -> v3:
> >  - QAPI style fixes
> >  - Typos fixes in comments
> >  - `#include`'s updated to be in sync with recent master
> > v3 -> v4:
> >  - Support vmnet interfaces isolation feature
> >  - Support vmnet-host network uuid setting feature
> >  - Refactored sources a bit
> > v4 -> v5:
> >  - Missed 6.2 boat, now 7.0 candidate
> >  - Fix qapi netdev descriptions and styles
> >(@subnetmask -> @subnet-mask)
> >  - Support vmnet-shared IPv6 prefix setting feature
> > v5 -> v6
> >  - provide detailed commit messages for commits of
> >many changes
> >  - rename properties @dhcpstart and @dhcpend to
> >@start-address and @end-address
> >  - improve qapi documentation about isolation
> >features (@isolated, @net-uuid)
> > v6 -> v7:
> >  - update MAINTAINERS list
> > v7 -> v8
> >  - QAPI code style fixes
> > v8 -> v9
> >  - Fix building on Linux: add missing qapi
> >`'if': 'CONFIG_VMNET'` statement to Netdev union
> > v9 -> v10
> >  - Disable vmnet feature for macOS < 11.0: add
> >vmnet.framework API probe into meson.build.
> >This fixes QEMU building on macOS < 11.0:
> >https://patchew.org/QEMU/20220110034000.20221-1-jasow...@redhat.com/
> > 
> 
> Hi Vladislav,
> 
> What symbols are missing on Catalina except VMNET_SHARING_BUSY?
> 
> It'd be great to get the feature working there.
> 
> Thanks,
> Roman
> 

Ok it turned out not that many symbols are needed for successfull
compilation on Catalina:

vmnet_enable_i

Re: [PATCH v10 0/7] Add vmnet.framework based network backend

2022-01-12 Thread Roman Bolshakov
On Wed, Jan 12, 2022 at 04:23:30PM +0300, Vladislav Yaroshchuk wrote:
> ср, 12 янв. 2022 г. в 11:22, Roman Bolshakov :
> 
> > On Wed, Jan 12, 2022 at 10:50:04AM +0300, Roman Bolshakov wrote:
> > > On Wed, Jan 12, 2022 at 12:14:15AM +0300, Vladislav Yaroshchuk wrote:
> > > > v9 -> v10
> > > >  - Disable vmnet feature for macOS < 11.0: add
> > > >vmnet.framework API probe into meson.build.
> > > >This fixes QEMU building on macOS < 11.0:
> > > >
> > >
> > > Hi Vladislav,
> > >
> > > What symbols are missing on Catalina except VMNET_SHARING_BUSY?
> > >
> > > It'd be great to get the feature working there.
> > >
> > > Thanks,
> > > Roman
> > >
> >
> > Ok it turned out not that many symbols are needed for successfull
> > compilation on Catalina:
> >
> > vmnet_enable_isolation_key
> > vmnet_network_identifier_key
> > VMNET_SHARING_SERVICE_BUSY
> >
> > The compilation suceeds if they're wrappeed by ifdefs. I haven't tested
> > it yet though.
> >
> >
> New version with Catalina 10.15 support submitted as v11.
> 

Thanks!

I appreciate that.

Regards,
Roman




Re: [PATCH v11 1/7] net/vmnet: add vmnet dependency and customizable option

2022-01-12 Thread Roman Bolshakov
On Wed, Jan 12, 2022 at 03:21:44PM +0300, Vladislav Yaroshchuk wrote:
> vmnet.framework dependency is added with 'vmnet' option
> to enable or disable it. Default value is 'auto'.
> 
> vmnet features to be used are available since macOS 11.0,
> corresponding probe is created into meson.build.
> 
> Signed-off-by: Vladislav Yaroshchuk 
> ---
>  meson.build   | 23 ++-
>  meson_options.txt |  2 ++
>  scripts/meson-buildoptions.sh |  3 +++
>  3 files changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/meson.build b/meson.build
> index c1b1db1e28..b912c9cb91 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -496,6 +496,24 @@ if cocoa.found() and get_option('gtk').enabled()
>error('Cocoa and GTK+ cannot be enabled at the same time')
>  endif
>  
> +vmnet = dependency('appleframeworks', modules: 'vmnet', required: 
> get_option('vmnet'))
> +vmnet_11_0_api = false
> +if vmnet.found() and not cc.has_header_symbol('vmnet/vmnet.h',
> +  'VMNET_BRIDGED_MODE',
> +  dependencies: vmnet)
> +  vmnet = not_found
> +  if get_option('vmnet').enabled()
> +error('vmnet.framework API is outdated')
> +  else
> +warning('vmnet.framework API is outdated, disabling')
> +  endif
> +endif
> +if vmnet.found() and cc.has_header_symbol('vmnet/vmnet.h',
> +  'VMNET_SHARING_SERVICE_BUSY',
> +  dependencies: vmnet)
> +  vmnet_11_0_api = true
> +endif
> +
>  seccomp = not_found
>  if not get_option('seccomp').auto() or have_system or have_tools
>seccomp = dependency('libseccomp', version: '>=2.3.0',
> @@ -1492,6 +1510,8 @@ config_host_data.set('CONFIG_SECCOMP', seccomp.found())
>  config_host_data.set('CONFIG_SNAPPY', snappy.found())
>  config_host_data.set('CONFIG_USB_LIBUSB', libusb.found())
>  config_host_data.set('CONFIG_VDE', vde.found())
> +config_host_data.set('CONFIG_VMNET', vmnet.found())
> +config_host_data.set('CONFIG_VMNET_11_0_API', vmnet_11_0_api)

Hi Vladislav,

There might be more functionality coming in the next macOS versions but
we likely don't want to add them as extra CONFIG defines. Instead we
wrap new symbols/functions/code that are avaialble above Big Sur in the
code as:

#if defined(MAC_OS_VERSION_11_0) && \
MAC_OS_X_VERSION_MAX_ALLOWED >= MAC_OS_VERSION_11_0

xpc_dictionary_set_bool(
if_desc,
vmnet_enable_isolation_key,
options->isolated
);

#endif

Please see similar thread here:
https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg01915.html

Thanks,
Roman

>  config_host_data.set('CONFIG_VHOST_USER_BLK_SERVER', 
> have_vhost_user_blk_server)
>  config_host_data.set('CONFIG_VNC', vnc.found())
>  config_host_data.set('CONFIG_VNC_JPEG', jpeg.found())
> @@ -3406,7 +3426,8 @@ summary(summary_info, bool_yn: true, section: 'Crypto')
>  # Libraries
>  summary_info = {}
>  if targetos == 'darwin'
> -  summary_info += {'Cocoa support':   cocoa}
> +  summary_info += {'Cocoa support':   cocoa}
> +  summary_info += {'vmnet.framework support': vmnet}
>  endif
>  summary_info += {'SDL support':   sdl}
>  summary_info += {'SDL image support': sdl_image}
> diff --git a/meson_options.txt b/meson_options.txt
> index 921967eddb..701e1381f9 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -151,6 +151,8 @@ option('netmap', type : 'feature', value : 'auto',
> description: 'netmap network backend support')
>  option('vde', type : 'feature', value : 'auto',
> description: 'vde network backend support')
> +option('vmnet', type : 'feature', value : 'auto',
> +   description: 'vmnet.framework network backend support')
>  option('virglrenderer', type : 'feature', value : 'auto',
> description: 'virgl rendering support')
>  option('vnc', type : 'feature', value : 'auto',
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index 50bd7bed4d..cdcece4b05 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -84,6 +84,7 @@ meson_options_help() {
>printf "%s\n" '  u2f U2F emulation support'
>printf "%s\n" '  usb-redir   libusbredir support'
>printf "%s\n" '  vde vde network backend support'
> +  printf "%s\n" '  vmnet   vmnet.framework network backend support'
>printf "%s\n" '  vhost-user-blk-server'
>printf "%s\n" '  build vhost-user-blk server'
>printf "%s\n" '  virglrenderer   virgl rendering support'
> @@ -248,6 +249,8 @@ _meson_option_parse() {
>  --disable-usb-redir) printf "%s" -Dusb_redir=disabled ;;
>  --enable-vde) printf "%s" -Dvde=enabled ;;
>  --disable-vde) printf "%s" -Dvde=disabled ;;
> +--enable-vmnet) printf "%s" -Dvmnet=enabled ;;
> +--disable-vmnet) printf "%s" -Dvmnet=disabled ;;
>  --enable-vhost-user-blk-server) printf "%s" 
> -Dvhost_use

Re: [PATCH v3 01/16] MAINTAINERS: Update Roman Bolshakov email address

2023-06-28 Thread Roman Bolshakov
24.06.2023 20:41, Philippe Mathieu-Daudé пишет:
> r.bolsha...@yadro.com is bouncing: Update Roman's email address
> using one found somewhere on the Internet; this way he can Ack-by.
>
> (Reorder Taylor's line to keep the section sorted alphabetically).
>
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>   MAINTAINERS | 4 ++--
>   .mailmap| 3 ++-
>   2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7f323cd2eb..1da135b0c8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -497,14 +497,14 @@ F: target/arm/hvf/
>   
>   X86 HVF CPUs
>   M: Cameron Esfahani 
> -M: Roman Bolshakov 
> +M: Roman Bolshakov 
>   W: https://wiki.qemu.org/Features/HVF
>   S: Maintained
>   F: target/i386/hvf/
>   
>   HVF
>   M: Cameron Esfahani 
> -M: Roman Bolshakov 
> +M: Roman Bolshakov 
>   W: https://wiki.qemu.org/Features/HVF
>   S: Maintained
>   F: accel/hvf/
> diff --git a/.mailmap b/.mailmap
> index b57da4827e..64ef9f4de6 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -76,9 +76,10 @@ Paul Burton  
>   Philippe Mathieu-Daudé  
>   Philippe Mathieu-Daudé  
>   Philippe Mathieu-Daudé  
> +Roman Bolshakov  
>   Stefan Brankovic  
> 
> -Yongbok Kim  
>   Taylor Simpson  
> +Yongbok Kim  
>   
>   # Also list preferred name forms where people have changed their
>   # git author config, or had utf8/latin1 encoding issues.

Hi Philippe,

Reviewed-by: Roman Bolshakov 

Thanks for updating the email.



Re: [Qemu-devel] [PATCH for-3.1 2/2] i386: hvf: drop debug printf in decode_sldtgroup

2018-12-12 Thread Roman Bolshakov
On Mon, Dec 03, 2018 at 01:04:15PM +0300, Roman Bolshakov wrote:
> It's going to clutter QEMU logs if 0x0f00 is trapped.
> 
> Signed-off-by: Roman Bolshakov 
> ---
>  target/i386/hvf/x86_decode.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
> index d125a6ef83..5f513c5563 100644
> --- a/target/i386/hvf/x86_decode.c
> +++ b/target/i386/hvf/x86_decode.c
> @@ -525,8 +525,6 @@ static void decode_sldtgroup(CPUX86State *env, struct 
> x86_decode *decode)
>  X86_DECODE_CMD_INVL
>  };
>  decode->cmd = group[decode->modrm.reg];
> -printf("%llx: decode_sldtgroup: %d\n", env->hvf_emul->fetch_rip,
> -decode->modrm.reg);
>  }
>  
>  static void decode_lidtgroup(CPUX86State *env, struct x86_decode *decode)
> -- 
> 2.17.2 (Apple Git-113)
> 

Could you please apply to trivial queue?

Thank you,
Roman



[Qemu-devel] [Bug 1798451] Re: MMX emulation is missing on HVF Acceleration

2018-11-19 Thread Roman Bolshakov
** Summary changed:

- HVF linux on OSX hangs 2nd time started after adding socket
+ MMX emulation is missing on HVF Acceleration

** Description changed:

- 
  Robs-MacBook-Pro-2:~ robmaskell$ qemu-system-x86_64 --version
  QEMU emulator version 3.0.0
  
  Host: MacOS - 10.13.6
Model Name: MacBook Pro
Model Identifier:   MacBookPro14,3
Processor Name: Intel Core i7
Processor Speed:2.8 GHz
Number of Processors:   1
Total Number of Cores:  4
L2 Cache (per Core):256 KB
L3 Cache:   6 MB
Memory: 16 GB
  
  Guest OS: Elementary Linux Loki 0.4.1, patched up to date
  
  Command used to start QEMU:
  
  qemu-system-x86_64 \
-name ElementaryLokiDev \
-machine pc,accel=hvf \
-cpu max \
-smp cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 \
-numa node,nodeid=0 \
-numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1 \
-m 8G \
-vga vmware \
-hda e4.qcow2
  
  Symptoms: Started without the -smp / -numa commands to install the OS,
  then added -smp / -numa and the machine boots and lscpu reports extra
  cpu as expected. Restart VM and it hangs on startup. Remove -smp / -numa
  and machine starts again.

** Tags added: hvf x86

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1798451

Title:
  MMX emulation is missing on HVF Acceleration

Status in QEMU:
  New

Bug description:
  Robs-MacBook-Pro-2:~ robmaskell$ qemu-system-x86_64 --version
  QEMU emulator version 3.0.0

  Host: MacOS - 10.13.6
Model Name: MacBook Pro
Model Identifier:   MacBookPro14,3
Processor Name: Intel Core i7
Processor Speed:2.8 GHz
Number of Processors:   1
Total Number of Cores:  4
L2 Cache (per Core):256 KB
L3 Cache:   6 MB
Memory: 16 GB

  Guest OS: Elementary Linux Loki 0.4.1, patched up to date

  Command used to start QEMU:

  qemu-system-x86_64 \
-name ElementaryLokiDev \
-machine pc,accel=hvf \
-cpu max \
-smp cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 \
-numa node,nodeid=0 \
-numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1 \
-m 8G \
-vga vmware \
-hda e4.qcow2

  Symptoms: Started without the -smp / -numa commands to install the OS,
  then added -smp / -numa and the machine boots and lscpu reports extra
  cpu as expected. Restart VM and it hangs on startup. Remove -smp /
  -numa and machine starts again.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1798451/+subscriptions



[Qemu-devel] modern virtio on HVF

2018-10-16 Thread Roman Bolshakov
Hello dear subscribers,

I'm running Linux in QEMU on macOS with hvf accel enabled and having an
issue that is very similar to the KVM bug in nested KVM environments,
where KVM is run under another hypervisor:
https://bugs.launchpad.net/qemu/+bug/1636217


The symptomps are the same as in the bug above. udev hangs unless:
* -machine type=pc-i440fx-X, where X <=2.6 is used
* -accel tcg is used
* -global virtio-pci.disable-modern=on is specified

The issue was briefly noted on packer mailing list:
https://groups.google.com/forum/#!topic/packer-tool/je2D0LRhWj0

If I send Magic SysRQ-t to the VM, I can notice virtio_pci hangs
indefinetly in vp_reset:
[   48.604482] systemd-udevd   D0   121106 0x0100
[   48.608093] Call Trace:
[   48.609701]  ? __schedule+0x292/0x880
[   48.612076]  schedule+0x32/0x80
[   48.614189]  schedule_timeout+0x15e/0x300
[   48.616840]  ? call_timer_fn+0x140/0x140
[   48.619375]  msleep+0x2a/0x40
[   48.621284]  vp_reset+0x27/0x50 [virtio_pci]
[   48.624185]  register_virtio_device+0x71/0x100 [virtio]
[   48.627689]  virtio_pci_probe+0xad/0x120 [virtio_pci]
[   48.630825]  local_pci_probe+0x44/0xa0
[   48.633357]  pci_device_probe+0x127/0x140
[   48.636085]  driver_probe_device+0x297/0x450
[   48.638876]  __driver_attach+0xd9/0xe0
[   48.641484]  ? driver_probe_device+0x450/0x450
[   48.644393]  bus_for_each_dev+0x5a/0x90
[   48.646879]  bus_add_driver+0x41/0x260
[   48.649279]  driver_register+0x5b/0xd0
[   48.651703]  ? 0xc00ac000
[   48.653994]  do_one_initcall+0x50/0x1b0
[   48.656496]  do_init_module+0x5a/0x1fa
[   48.659001]  load_module+0x1557/0x1ed0
[   48.661507]  ? m_show+0x1b0/0x1b0
[   48.663725]  ? security_capable+0x47/0x60
[   48.666435]  SYSC_finit_module+0x80/0xb0
[   48.669021]  do_syscall_64+0x74/0x150
[   48.671222]  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
[   48.674565] RIP: 0033:0x7f71dac73139
[   48.677050] RSP: 002b:7ffdcfd35058 EFLAGS: 0246 ORIG_RAX: 
0139
[   48.681997] RAX: ffda RBX: 55d25bd01500 RCX: 7f71dac73139
[   48.686608] RDX:  RSI: 7f71db5af83d RDI: 000f
[   48.691191] RBP: 7f71db5af83d R08:  R09: 
[   48.695750] R10: 000f R11: 0246 R12: 0002
[   48.700568] R13: 55d25bd039b0 R14:  R15: 03938700

It looks like virtio backend doesn't return 0 device status after
vp_iowrite8 and vp_reset blocks udev:
while (vp_ioread8(&vp_dev->common->device_status))
msleep(1)

What could be the cause of the issue?
Any advices how to triage it are appreciated.

Thank you,
Roman



Re: [Qemu-devel] modern virtio on HVF

2018-10-17 Thread Roman Bolshakov
On Wed, Oct 17, 2018 at 10:47:40AM +0100, Stefan Hajnoczi wrote:
> On Tue, Oct 16, 2018 at 06:27:12PM +0300, Roman Bolshakov wrote:
> > 
> > It looks like virtio backend doesn't return 0 device status after
> > vp_iowrite8 and vp_reset blocks udev:
> > while (vp_ioread8(&vp_dev->common->device_status))
> > msleep(1)
> > 
> > What could be the cause of the issue?
> > Any advices how to triage it are appreciated.
> 
> I wonder what happened in virtio_pci_probe() ->
> virtio_pci_modern_probe().  For example, were the BARs properly set up?
> 
> For starters you can debug the QEMU process to check if
> virtio_pci_common_read/write() get called for this device (disable all
> other virtio devices to make life easy).  If these functions aren't
> being called then the guest either got the address wrong or dispatch
> isn't working for some other reason (hvf?).
> 

Thank you Stefan. That explains why it doesn't quit:

virtio_pci_common_write: hwaddr 0x14 val 0x8 size 0x1
38552@1539791595.072718:virtio_set_status vdev 0x7f92d5ef4170 val 8
virtio_pci_common_read: hwaddr 0x14 val 0x8 size 0x1
virtio_pci_common_read: hwaddr 0x14 val 0x8 size 0x1
virtio_pci_common_read: hwaddr 0x14 val 0x8 size 0x1
virtio_pci_common_read: hwaddr 0x14 val 0x8 size 0x1
virtio_pci_common_read: hwaddr 0x14 val 0x8 size 0x1
virtio_pci_common_read: hwaddr 0x14 val 0x8 size 0x1


I executed QEMU a number of times, each time vpirtio_pci_common_write
writes different value but virtio_pci code clearly writes 0:
/* 0 status means a reset. */
vp_iowrite8(0, &vp_dev->common->device_status);

Maybe the value somehow got corrupted or it was taken from wrong location.

-Roman



[Qemu-devel] [PATCH] i386: hvf: Fix register refs if REX is present

2018-10-18 Thread Roman Bolshakov
According to Intel(R)64 and IA-32 Architectures Software Developer's
Manual, the following one-byte registers should be fetched when REX
prefix is present (sorted by reg encoding index):
AL, CL, DL, BL, SPL, BPL, SIL, DIL, R8L - R15L

The first 8 are fetched if REX.R is zero, the last 8 if non-zero.

The following registers should be fetched for instructions without REX
prefix (also sorted by reg encoding index):
AL, CL, DL, BL, AH, CH, DH, BH

Current emulation code doesn't handle accesses to SPL, BPL, SIL, DIL
when REX is present, thefore an instruction 40883e "mov %dil,(%rsi)" is
decoded as "mov %bh,(%rsi)".

That caused an infinite loop in vp_reset:
https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg03293.html

Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/x86_decode.c | 67 
 target/i386/hvf/x86_decode.h |  6 ++--
 2 files changed, 42 insertions(+), 31 deletions(-)

diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
index 2d7540fe7c..2e33b69541 100644
--- a/target/i386/hvf/x86_decode.c
+++ b/target/i386/hvf/x86_decode.c
@@ -113,7 +113,8 @@ static void decode_modrm_reg(CPUX86State *env, struct 
x86_decode *decode,
 {
 op->type = X86_VAR_REG;
 op->reg = decode->modrm.reg;
-op->ptr = get_reg_ref(env, op->reg, decode->rex.r, decode->operand_size);
+op->ptr = get_reg_ref(env, op->reg, decode->rex.rex, decode->rex.r,
+  decode->operand_size);
 }
 
 static void decode_rax(CPUX86State *env, struct x86_decode *decode,
@@ -121,7 +122,8 @@ static void decode_rax(CPUX86State *env, struct x86_decode 
*decode,
 {
 op->type = X86_VAR_REG;
 op->reg = R_EAX;
-op->ptr = get_reg_ref(env, op->reg, 0, decode->operand_size);
+op->ptr = get_reg_ref(env, op->reg, decode->rex.rex, 0,
+  decode->operand_size);
 }
 
 static inline void decode_immediate(CPUX86State *env, struct x86_decode 
*decode,
@@ -263,16 +265,16 @@ static void decode_incgroup(CPUX86State *env, struct 
x86_decode *decode)
 {
 decode->op[0].type = X86_VAR_REG;
 decode->op[0].reg = decode->opcode[0] - 0x40;
-decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.b,
-decode->operand_size);
+decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.rex,
+decode->rex.b, decode->operand_size);
 }
 
 static void decode_decgroup(CPUX86State *env, struct x86_decode *decode)
 {
 decode->op[0].type = X86_VAR_REG;
 decode->op[0].reg = decode->opcode[0] - 0x48;
-decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.b,
-decode->operand_size);
+decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.rex,
+decode->rex.b, decode->operand_size);
 }
 
 static void decode_incgroup2(CPUX86State *env, struct x86_decode *decode)
@@ -288,16 +290,16 @@ static void decode_pushgroup(CPUX86State *env, struct 
x86_decode *decode)
 {
 decode->op[0].type = X86_VAR_REG;
 decode->op[0].reg = decode->opcode[0] - 0x50;
-decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.b,
-decode->operand_size);
+decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.rex,
+decode->rex.b, decode->operand_size);
 }
 
 static void decode_popgroup(CPUX86State *env, struct x86_decode *decode)
 {
 decode->op[0].type = X86_VAR_REG;
 decode->op[0].reg = decode->opcode[0] - 0x58;
-decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.b,
-decode->operand_size);
+decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.rex,
+decode->rex.b, decode->operand_size);
 }
 
 static void decode_jxx(CPUX86State *env, struct x86_decode *decode)
@@ -378,16 +380,16 @@ static void decode_xchgroup(CPUX86State *env, struct 
x86_decode *decode)
 {
 decode->op[0].type = X86_VAR_REG;
 decode->op[0].reg = decode->opcode[0] - 0x90;
-decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.b,
-decode->operand_size);
+decode->op[0].ptr = get_reg_ref(env, decode->op[0].reg, decode->rex.rex,
+decode->rex.b, decode->operand_size);
 }
 
 static void decode_movgroup(CPUX86State *env, struct x86_decode *decode)
 {
 decode->op[0].type = X86_VAR_REG;
 decode->op[0].reg = decode->opcode[0] - 0xb8;
-decode->op[0].ptr = get_reg_ref(env, decode->op[0].re

[Qemu-devel] [PATCH] i386: hvf: Remove hvf_disabled

2018-10-18 Thread Roman Bolshakov
accel_init_machine sets *(acc->allowed) to true if acc->init_machine(ms)
succeeds. There's no need to have both hvf_allowed and hvf_disabled.

Signed-off-by: Roman Bolshakov 
---
 include/sysemu/hvf.h  | 4 ++--
 target/i386/hvf/hvf.c | 9 +
 2 files changed, 3 insertions(+), 10 deletions(-)

diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index 241118845c..aaa51d2c51 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -17,7 +17,7 @@
 #include "exec/memory.h"
 #include "sysemu/accel.h"
 
-extern int hvf_disabled;
+extern bool hvf_allowed;
 #ifdef CONFIG_HVF
 #include 
 #include 
@@ -26,7 +26,7 @@ extern int hvf_disabled;
 #include "hw/hw.h"
 uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t idx,
  int reg);
-#define hvf_enabled() !hvf_disabled
+#define hvf_enabled() (hvf_allowed)
 #else
 #define hvf_enabled() 0
 #define hvf_get_supported_cpuid(func, idx, reg) 0
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 9f52bc413a..e193022c03 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -73,7 +73,6 @@
 #include "target/i386/cpu.h"
 
 HVFState *hvf_state;
-int hvf_disabled = 1;
 
 static void assert_hvf_ok(hv_return_t ret)
 {
@@ -604,11 +603,6 @@ int hvf_init_vcpu(CPUState *cpu)
 return 0;
 }
 
-void hvf_disable(int shouldDisable)
-{
-hvf_disabled = shouldDisable;
-}
-
 static void hvf_store_events(CPUState *cpu, uint32_t ins_len, uint64_t 
idtvec_info)
 {
 X86CPU *x86_cpu = X86_CPU(cpu);
@@ -934,7 +928,7 @@ int hvf_vcpu_exec(CPUState *cpu)
 return ret;
 }
 
-static bool hvf_allowed;
+bool hvf_allowed;
 
 static int hvf_accel_init(MachineState *ms)
 {
@@ -942,7 +936,6 @@ static int hvf_accel_init(MachineState *ms)
 hv_return_t ret;
 HVFState *s;
 
-hvf_disable(0);
 ret = hv_vm_create(HV_VM_DEFAULT);
 assert_hvf_ok(ret);
 
-- 
2.17.1 (Apple Git-112)




[Qemu-devel] [Bug 1798451] Re: HVF linux on OSX hangs 2nd time started after adding socket

2018-10-20 Thread Roman Bolshakov
I've had issues with multiple vcpus previously.

But I've tried that recently and it worked fine with the fix:
https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg03864.html.

And I've checked your command, no issues.

Could you please try to install qemu from my tap and check if it's gone?

brew tap roolebo/virt
brew install roolebo/virt/qemu --HEAD

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1798451

Title:
  HVF linux on OSX hangs 2nd time started after adding socket

Status in QEMU:
  New

Bug description:
  
  Robs-MacBook-Pro-2:~ robmaskell$ qemu-system-x86_64 --version
  QEMU emulator version 3.0.0

  Host: MacOS - 10.13.6
Model Name: MacBook Pro
Model Identifier:   MacBookPro14,3
Processor Name: Intel Core i7
Processor Speed:2.8 GHz
Number of Processors:   1
Total Number of Cores:  4
L2 Cache (per Core):256 KB
L3 Cache:   6 MB
Memory: 16 GB

  Guest OS: Elementary Linux Loki 0.4.1, patched up to date

  Command used to start QEMU:

  qemu-system-x86_64 \
-name ElementaryLokiDev \
-machine pc,accel=hvf \
-cpu max \
-smp cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 \
-numa node,nodeid=0 \
-numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1 \
-m 8G \
-vga vmware \
-hda e4.qcow2

  Symptoms: Started without the -smp / -numa commands to install the OS,
  then added -smp / -numa and the machine boots and lscpu reports extra
  cpu as expected. Restart VM and it hangs on startup. Remove -smp /
  -numa and machine starts again.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1798451/+subscriptions



[Qemu-devel] [Bug 1798451] Re: HVF linux on OSX hangs 2nd time started after adding socket

2018-10-22 Thread Roman Bolshakov
I have tried to run the OS and I can confirm that some instructions that
require VMEXIT are not implemented. In your case that's 0F7F or MOVQ
(mem from mmxreg) from MMX. In my case that's 0F11 or MOVUPS(xmmreg1 to
mem) from SSE.

I'd recommend you to run -cpu host,-mmx,-sse for a while, but the kernel
of the OS explicitly complains that it won't run on CPUs without SSE
support.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1798451

Title:
  HVF linux on OSX hangs 2nd time started after adding socket

Status in QEMU:
  New

Bug description:
  
  Robs-MacBook-Pro-2:~ robmaskell$ qemu-system-x86_64 --version
  QEMU emulator version 3.0.0

  Host: MacOS - 10.13.6
Model Name: MacBook Pro
Model Identifier:   MacBookPro14,3
Processor Name: Intel Core i7
Processor Speed:2.8 GHz
Number of Processors:   1
Total Number of Cores:  4
L2 Cache (per Core):256 KB
L3 Cache:   6 MB
Memory: 16 GB

  Guest OS: Elementary Linux Loki 0.4.1, patched up to date

  Command used to start QEMU:

  qemu-system-x86_64 \
-name ElementaryLokiDev \
-machine pc,accel=hvf \
-cpu max \
-smp cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 \
-numa node,nodeid=0 \
-numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1 \
-m 8G \
-vga vmware \
-hda e4.qcow2

  Symptoms: Started without the -smp / -numa commands to install the OS,
  then added -smp / -numa and the machine boots and lscpu reports extra
  cpu as expected. Restart VM and it hangs on startup. Remove -smp /
  -numa and machine starts again.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1798451/+subscriptions



[Qemu-devel] [Bug 1798451] Re: HVF linux on OSX hangs 2nd time started after adding socket

2018-10-23 Thread Roman Bolshakov
Considering the fact that both Ubuntu and Elementary require SSE to
boot, I'd wait to get decoding fixed. I wrote a test kernel module that
reliably reproduces your issue on qemu edu device. Whenever QEMU prints
Unimplemented handler Instruction pointer only moves two bytes further,
instead of the instruction length. That corrupts code execution as the
next instruction after unimplemented handler is decoded from the wrong
address.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1798451

Title:
  HVF linux on OSX hangs 2nd time started after adding socket

Status in QEMU:
  New

Bug description:
  
  Robs-MacBook-Pro-2:~ robmaskell$ qemu-system-x86_64 --version
  QEMU emulator version 3.0.0

  Host: MacOS - 10.13.6
Model Name: MacBook Pro
Model Identifier:   MacBookPro14,3
Processor Name: Intel Core i7
Processor Speed:2.8 GHz
Number of Processors:   1
Total Number of Cores:  4
L2 Cache (per Core):256 KB
L3 Cache:   6 MB
Memory: 16 GB

  Guest OS: Elementary Linux Loki 0.4.1, patched up to date

  Command used to start QEMU:

  qemu-system-x86_64 \
-name ElementaryLokiDev \
-machine pc,accel=hvf \
-cpu max \
-smp cpus=2,sockets=2,cores=1,threads=1,maxcpus=2 \
-numa node,nodeid=0 \
-numa cpu,node-id=0,socket-id=0 -numa cpu,node-id=0,socket-id=1 \
-m 8G \
-vga vmware \
-hda e4.qcow2

  Symptoms: Started without the -smp / -numa commands to install the OS,
  then added -smp / -numa and the machine boots and lscpu reports extra
  cpu as expected. Restart VM and it hangs on startup. Remove -smp /
  -numa and machine starts again.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1798451/+subscriptions



[Qemu-devel] [PATCH] roms: seabios: Rename CROSS_COMPILE to CROSS_PREFIX

2018-11-21 Thread Roman Bolshakov
SeaBIOS introduced CROSS_PREFIX in 2013 but it's not set in roms
Makefile.

With the change it's possible to cross-compile SeaBIOS on macOS,
if acpica/iasl is installed:
  cd roms
  export PATH=/path/to/cross/x86_64-unknown-linux-gnu/bin:$PATH
  make bios system=unknown-linux-gnu

Signed-off-by: Roman Bolshakov 
---
 roms/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/roms/Makefile b/roms/Makefile
index f4141e1d96..a6043eff37 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -83,12 +83,12 @@ build-seabios-config-%: config.%
cp $< seabios/builds/$*/.config
$(MAKE) -C seabios \
EXTRAVERSION=$(SEABIOS_EXTRAVERSION) \
-   CROSS_COMPILE=$(x86_64_cross_prefix) \
+   CROSS_PREFIX=$(x86_64_cross_prefix) \
KCONFIG_CONFIG=$(CURDIR)/seabios/builds/$*/.config \
OUT=$(CURDIR)/seabios/builds/$*/ oldnoconfig
$(MAKE) -C seabios \
EXTRAVERSION=$(SEABIOS_EXTRAVERSION) \
-   CROSS_COMPILE=$(x86_64_cross_prefix) \
+   CROSS_PREFIX=$(x86_64_cross_prefix) \
KCONFIG_CONFIG=$(CURDIR)/seabios/builds/$*/.config \
OUT=$(CURDIR)/seabios/builds/$*/ all
 
-- 
2.19.1




Re: [Qemu-devel] [RFC 41/48] configure: add --enable-plugins

2018-11-27 Thread Roman Bolshakov
On Thu, Oct 25, 2018 at 01:20:50PM -0400, Emilio G. Cota wrote:
> For now only add it for ELF platforms, since we rely on the linker's
> --dynamic-list flag to pass a list of symbols to be exported to the
> executable. An alternative would be to use -rdynamic, but that would
> expose all of QEMU's objects to plugins.
> 
> I have no experience with non-ELF systems but I suspect adding support
> for those should be pretty easy.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  configure | 51 +++
>  1 file changed, 51 insertions(+)
> 
> diff --git a/configure b/configure
> index 03bf719ca7..78e86098e5 100755
> --- a/configure
> +++ b/configure
> @@ -30,6 +30,7 @@ TMPO="${TMPDIR1}/${TMPB}.o"
>  TMPCXX="${TMPDIR1}/${TMPB}.cxx"
>  TMPE="${TMPDIR1}/${TMPB}.exe"
>  TMPMO="${TMPDIR1}/${TMPB}.mo"
> +TMPTXT="${TMPDIR1}/${TMPB}.txt"
>  
>  rm -f config.log
>  
> @@ -477,6 +478,7 @@ libxml2=""
>  docker="no"
>  debug_mutex="no"
>  libpmem=""
> +plugins="no"
>  
>  # cross compilers defaults, can be overridden with --cross-cc-ARCH
>  cross_cc_aarch64="aarch64-linux-gnu-gcc"
> @@ -1443,6 +1445,10 @@ for opt do
>;;
>--disable-libpmem) libpmem=no
>;;
> +  --enable-plugins) plugins="yes"
> +  ;;
> +  --disable-plugins) plugins="no"
> +  ;;
>*)
>echo "ERROR: unknown option $opt"
>echo "Try '$0 --help' for more information"
> @@ -1633,6 +1639,8 @@ Advanced options (experts only):
> xen pv domain builder
>--enable-debug-stack-usage
> track the maximum stack usage of stacks created 
> by qemu_alloc_stack
> +  --enable-plugins
> +   enable plugins via shared library loading
>  
>  Optional features, enabled with --enable-FEATURE and
>  disabled with --disable-FEATURE, default is enabled if available:
> @@ -5204,6 +5212,42 @@ if compile_prog "" "" ; then
>atomic64=yes
>  fi
>  
> +#
> +# See if --dynamic-list is supported by the linker
> +
> +cat > $TMPTXT < +{
> +  foo;
> +};
> +EOF
> +
> +cat > $TMPC < +#include 
> +void foo(void);
> +
> +void foo(void)
> +{
> +  printf("foo\n");
> +}
> +
> +int main(void)
> +{
> +  foo();
> +  return 0;
> +}
> +EOF
> +
> +if compile_prog "" "-Wl,--dynamic-list=$TMPTXT" ; then
> +  ld_dynamic_list="yes"
> +else
> +  if test "$plugins" = "yes" ; then
> +error_exit \
> +"Plugin support requires specifying a set of symbols that " \
> +"are exported to plugins. Unfortunately your linker doesn't " \
> +"support the flag (--dynamic-list) used for this purpose."
> +  fi
> +fi
> +
>  
>  # See if 16-byte vector operations are supported.
>  # Even without a vector unit the compiler may expand these.
> @@ -6091,6 +6135,7 @@ echo "VxHS block device $vxhs"
>  echo "capstone  $capstone"
>  echo "docker$docker"
>  echo "libpmem support   $libpmem"
> +echo "plugin support$plugins"
>  
>  if test "$sdl_too_old" = "yes"; then
>  echo "-> Your SDL version is too old - please upgrade to have SDL support"
> @@ -6849,6 +6894,12 @@ if test "$libpmem" = "yes" ; then
>echo "CONFIG_LIBPMEM=y" >> $config_host_mak
>  fi
>  
> +if test "$plugins" = "yes" ; then
> +echo "CONFIG_PLUGINS=y" >> $config_host_mak
> +LIBS="-ldl $LIBS"
> +LDFLAGS="-Wl,--dynamic-list=\$(SRC_PATH)/qemu-plugins.symbols $LDFLAGS"
> +fi
> +
>  if test "$tcg_interpreter" = "yes"; then
>QEMU_INCLUDES="-iquote \$(SRC_PATH)/tcg/tci $QEMU_INCLUDES"
>  elif test "$ARCH" = "sparc64" ; then
> -- 
> 2.17.1
> 
> 

ld64 on macOS has similar -exported_symbols_list option. Here's the reference:

 -exported_symbols_list filename
The specified filename contains a list of global symbol names
that will remain as global symbols in the output file.  All other
global symbols will be treated as if they were marked as
__private_extern__ (aka visibility=hidden) and will not be global in
the output file. The symbol names listed in filename must be one per
line.  Leading and trailing white space are not part of the symbol
name.  Lines starting with # are ignored, as are lines with only white
space.  Some wildcards (similar to shell file matching) are supported.
The * matches zero or more characters.  The ?  matches one character.
[abc] matches one character which must be an 'a', 'b', or 'c'.
[a-z] matches any single lower case letter from 'a' to 'z'.


I can try your branch if you add support of the linker flag or send required
changes via GitHub.

Best regards,
Roman



Re: [Qemu-devel] [RFC 41/48] configure: add --enable-plugins

2018-11-28 Thread Roman Bolshakov
On Tue, Nov 27, 2018 at 06:13:57PM -0500, Emilio G. Cota wrote:
> On Tue, Nov 27, 2018 at 15:43:52 +0300, Roman Bolshakov wrote:
> > ld64 on macOS has similar -exported_symbols_list option. Here's the 
> > reference:
> > 
> >  -exported_symbols_list filename
> > The specified filename contains a list of global symbol names
> > that will remain as global symbols in the output file.  All other
> > global symbols will be treated as if they were marked as
> > __private_extern__ (aka visibility=hidden) and will not be global in
> > the output file. The symbol names listed in filename must be one per
> > line.  Leading and trailing white space are not part of the symbol
> > name.  Lines starting with # are ignored, as are lines with only white
> > space.  Some wildcards (similar to shell file matching) are supported.
> > The * matches zero or more characters.  The ?  matches one character.
> > [abc] matches one character which must be an 'a', 'b', or 'c'.
> > [a-z] matches any single lower case letter from 'a' to 'z'.
> > 
> > 
> > I can try your branch if you add support of the linker flag or send required
> > changes via GitHub.
> 
> Can you please try this branch? I added a commit with the appended.
>   https://github.com/cota/qemu/tree/plugin-v2
> 
> You can inspect the symbols in the final binary with
> `readelf --dyn-syms' or `nm -D'.
> 
> Thanks,
> 
>   Emilio
> 
> ---
> diff --git a/configure b/configure
> index fe9707d951..3dc9c9697b 100755
> --- a/configure
> +++ b/configure
> @@ -5176,15 +5176,31 @@ int main(void)
>  }
>  EOF
>  
> +ld_dynamic_list="no"
>  if compile_prog "" "-Wl,--dynamic-list=$TMPTXT" ; then
>ld_dynamic_list="yes"
> -else
> -  if test "$plugins" = "yes" ; then
> -error_exit \
> -"Plugin support requires specifying a set of symbols that " \
> -"are exported to plugins. Unfortunately your linker doesn't " \
> -"support the flag (--dynamic-list) used for this purpose."
> -  fi
> +fi
> +
> +#
> +# See if -exported_symbols_list is supported by the linker
> +
> +cat > $TMPTXT < +  foo
> +EOF
> +
> +ld_exported_symbols_list="no"
> +if compile_prog "" "-Wl,-exported_symbols_list,$TMPTXT" ; then
> +  ld_exported_symbols_list="yes"
> +fi
> +
> +if  test "$plugins" = "yes" &&
> +test "$ld_dynamic_list" = "no" &&
> +test "$ld_exported_symbols_list" = "no" ; then
> +  error_exit \
> +  "Plugin support requires specifying a set of symbols that " \
> +  "are exported to plugins. Unfortunately your linker doesn't " \
> +  "support the flag (--dynamic-list or -exported_symbols_list) used " \
> +  "for this purpose."
>  fi
>  
>  
> @@ -6827,7 +6843,18 @@ fi
>  if test "$plugins" = "yes" ; then
>  echo "CONFIG_PLUGINS=y" >> $config_host_mak
>  LIBS="-ldl $LIBS"
> -LDFLAGS="-Wl,--dynamic-list=\$(SRC_PATH)/qemu-plugins.symbols $LDFLAGS"
> +if test "$ld_dynamic_list" = "yes" ; then
> + LDFLAGS="-Wl,--dynamic-list=\$(SRC_PATH)/qemu-plugins.symbols $LDFLAGS"
> +elif test "$ld_exported_symbols_list" = "yes" ; then
> + ld64_symbols=qemu-plugins-ld64.symbols
> + echo "# Automatically generated by configure - do not modify" > 
> $ld64_symbols
> + cat "$source_path/qemu-plugins.symbols" | grep qemu_ | sed 's/;//g' >> 
> $ld64_symbols
> + LDFLAGS="-Wl,-exported_symbols_list,\$(BUILD_DIR)/$ld64_symbols 
> $LDFLAGS"
> +else
> + error_exit \
> + "If \$plugins=yes, either \$ld_dynamic_list or " \
> + "\$ld_exported_symbols_list should have been set to 'yes'."
> +fi
>  fi
>  
>  if test "$tcg_interpreter" = "yes"; then
> 

Hi Emilio,

The test of -exported_symbols_list fails:
Undefined symbols for architecture x86_64:
  "foo", referenced from:
   -exported_symbol[s_list] command line option
(maybe you meant: _foo)

All functions on macOS are prefixed with underscore [1]:
"The name of a symbol representing a function that conforms to standard C
calli

Re: [Qemu-devel] [RFC 41/48] configure: add --enable-plugins

2018-11-29 Thread Roman Bolshakov
On Wed, Nov 28, 2018 at 12:23:32PM -0500, Emilio G. Cota wrote:
> On Wed, Nov 28, 2018 at 13:43:30 +0300, Roman Bolshakov wrote:
> > qemu-ga fails to link because it doesn't have symbols declared in
> > qemu-plugins-ld64.symbols. Perhaps "-Wl,-exported_symbols_list" should
> > be applied only to qemu-system?
> 
> I just pushed to the same github branch the appended fixup.
> The idea is to only add the linker flags when the output
> binary links plugin.o.
> 
> Can you please test?
> 
> Thanks,
> 
>   Emilio
> 
> ---
> commit 8f45416b79765b66e5ce0fca7db93b97bbcfcfbb
> Author: Emilio G. Cota 
> Date:   Wed Nov 28 12:11:23 2018 -0500
> 
> configure: ld64 fixup
> 
> And copy the file to the build dir also for !ld64.
> 
> Signed-off-by: Emilio G. Cota 
> 
> diff --git a/Makefile.target b/Makefile.target
> index 719699696d..7ea17d71cb 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -107,7 +107,23 @@ obj-y += target/$(TARGET_BASE_ARCH)/
>  obj-y += disas.o
>  obj-$(call notempty,$(TARGET_XML_FILES)) += gdbstub-xml.o
>  
> -obj-$(CONFIG_PLUGINS) += plugin.o
> +ifdef CONFIG_PLUGINS
> +obj-y += plugin.o
> +# Abuse -libs suffix to only link with --dynamic-list/-exported_symbols_list
> +# when the final binary includes the plugin object.
> +#
> +# Note that simply setting LDFLAGS is not enough: we build binaries that
> +# never link plugin.o, and the linker might fail (at least ld64 does)
> +# if the symbols in the list are not in the output binary.
> + ifdef CONFIG_HAS_LD_DYNAMIC_LIST
> + plugin.o-libs := -Wl,--dynamic-list=$(BUILD_DIR)/qemu-plugins-ld.symbols
> + else
> +  ifdef CONFIG_HAS_LD_EXPORTED_SYMBOLS_LIST
> +  plugin.o-libs := \
> + -Wl,-exported_symbols_list,$(BUILD_DIR)/qemu-plugins-ld64.symbols
> +  endif
> + endif
> +endif
>  
>  #
>  # Linux user emulator target
> diff --git a/configure b/configure
> index 3dc9c9697b..395acf831e 100755
> --- a/configure
> +++ b/configure
> @@ -5185,7 +5185,7 @@ fi
>  # See if -exported_symbols_list is supported by the linker
>  
>  cat > $TMPTXT < -  foo
> +  _foo
>  EOF
>  
>  ld_exported_symbols_list="no"
> @@ -6843,13 +6843,17 @@ fi
>  if test "$plugins" = "yes" ; then
>  echo "CONFIG_PLUGINS=y" >> $config_host_mak
>  LIBS="-ldl $LIBS"
> +# Copy the export object list to the build dir
>  if test "$ld_dynamic_list" = "yes" ; then
> - LDFLAGS="-Wl,--dynamic-list=\$(SRC_PATH)/qemu-plugins.symbols $LDFLAGS"
> + echo "CONFIG_HAS_LD_DYNAMIC_LIST=yes" >> $config_host_mak
> + ld_symbols=qemu-plugins-ld.symbols
> + cp "$source_path/qemu-plugins.symbols" $ld_symbols
>  elif test "$ld_exported_symbols_list" = "yes" ; then
> + echo "CONFIG_HAS_LD_EXPORTED_SYMBOLS_LIST=yes" >> $config_host_mak
>   ld64_symbols=qemu-plugins-ld64.symbols
>   echo "# Automatically generated by configure - do not modify" > 
> $ld64_symbols
> - cat "$source_path/qemu-plugins.symbols" | grep qemu_ | sed 's/;//g' >> 
> $ld64_symbols
> - LDFLAGS="-Wl,-exported_symbols_list,\$(BUILD_DIR)/$ld64_symbols 
> $LDFLAGS"
> + grep 'qemu_' "$source_path/qemu-plugins.symbols" | sed 's/;//g' | \
> + sed -E 's/^\s*(.*)/_\1/' >> $ld64_symbols
>  else
>   error_exit \
>   "If \$plugins=yes, either \$ld_dynamic_list or " \
> 
Hi Emilio,

I think there's an issue with "\s" character class, it's not recognized
by macOS sed  and I'm getting incorrect lines in
qemu-plugins-ld64.symbols:
_  qemu_xxx
_  qemu_xyz

After I replaced "\s" with "[[:space:]]", linking proceeds further, but
doesn't succeed because of an unresolved reference for qemu-system cris,
lm32, m68k, microblaze, microblazeel, moxie, nios2, or1k, riscv32,
riscv64, sparc, unicore32, tricore, xtensa, xtensaeb:

Undefined symbols for architecture x86_64:
  "_pci_register_bar", referenced from:
  _plugin_chan_realize in plugin-chan.o

It probably has nothing to do with macOS per-se and shouldn't link on
Linux as well. If I disable the aforementioned targets the build
succeeds and I can see the symbols from qemu-plugins-ld64.symbols in
compiled qemu-system binaries.

Best regards,
Roman



[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave

2018-11-29 Thread Roman Bolshakov
Can you try to build it without SDL/GTK support? I'm not having any
issues with Cocoa display.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1802684

Title:
  QEMU gui crashes on macOS Mojave

Status in QEMU:
  New

Bug description:
  QEMU release 3.0.0 as well as a recent head build

  /usr/local/Cellar/qemu/HEAD-03c1ca1 (147 files, 257.2MB)
Built from source on 2018-11-06 at 13:41:32 with: --with-gtk+3 --with-sdl2 
--with-libusb
  /usr/local/Cellar/qemu/3.0.0 (137 files, 261.6MB) *
Poured from bottle on 2018-11-10 at 22:58:32 with: --with-gtk+3 
--with-libusb --with-sdl2

  Crashes when attempting to use any gui interface (tried SDL and
  default Cocoa):

  2018-11-10 22:58:41.799 qemu-system-aarch64[42982:1102466] *** Terminating 
app due to uncaught exception 'NSInternalInconsistencyException', reason: 
'NSWindow drag regions should only be invalidated on the Main Thread!'
  *** First throw call stack:
  (
0   CoreFoundation  0x7fff3ea96ecd 
__exceptionPreprocess + 256
1   libobjc.A.dylib 0x7fff6ab49720 
objc_exception_throw + 48
2   CoreFoundation  0x7fff3eab095d 
-[NSException raise] + 9
3   AppKit  0x7fff3bfb13fa 
-[NSWindow(NSWindow_Theme) 
_postWindowNeedsToResetDragMarginsUnlessPostingDisabled] + 324
4   AppKit  0x7fff3bfb6850 -[NSView 
setFrameSize:] + 2082
5   AppKit  0x7fff3c02747d 
-[NSVisualEffectView setFrameSize:] + 171
6   AppKit  0x7fff3c0811b1 
-[NSTitlebarView setFrameSize:] + 84
7   AppKit  0x7fff3bfb5859 -[NSView 
setFrame:] + 478
8   AppKit  0x7fff3c081154 
-[NSTitlebarView resizeWithOldSuperviewSize:] + 100
9   AppKit  0x7fff3bfbc95e -[NSView 
resizeSubviewsWithOldSize:] + 502
10  AppKit  0x7fff3bfb66d9 -[NSView 
setFrameSize:] + 1707
11  AppKit  0x7fff3c9773c0 
-[NSTitlebarContainerView setFrameSize:] + 142
12  AppKit  0x7fff3bfb5859 -[NSView 
setFrame:] + 478
13  AppKit  0x7fff3bfbcdb5 -[NSView 
resizeWithOldSuperviewSize:] + 776
14  AppKit  0x7fff3bfbc95e -[NSView 
resizeSubviewsWithOldSize:] + 502
15  AppKit  0x7fff3bfb66d9 -[NSView 
setFrameSize:] + 1707
16  AppKit  0x7fff3c024570 
-[NSThemeFrame setFrameSize:] + 495
17  AppKit  0x7fff3c011223 -[NSWindow 
_setFrame:updateBorderViewSize:] + 966
18  AppKit  0x7fff3c010b46 -[NSWindow 
_oldPlaceWindow:] + 547
19  AppKit  0x7fff3c010151 -[NSWindow 
_setFrameCommon:display:stashSize:] + 3006
20  AppKit  0x7fff3c00f57d -[NSWindow 
_setFrame:display:allowImplicitAnimation:stashSize:] + 192
21  AppKit  0x7fff3c019ff8 -[NSWindow 
setFrame:display:animate:] + 567
22  qemu-system-aarch64 0x00010b7b2abf 
qemu-system-aarch64 + 3668671
23  qemu-system-aarch64 0x00010b7b6356 
qemu-system-aarch64 + 3683158
24  qemu-system-aarch64 0x00010b7ad836 
qemu-system-aarch64 + 3647542
25  qemu-system-aarch64 0x00010b4ce769 
qemu-system-aarch64 + 636777
26  qemu-system-aarch64 0x00010b487c24 
qemu-system-aarch64 + 347172
27  qemu-system-aarch64 0x00010b487a15 
qemu-system-aarch64 + 346645
28  qemu-system-aarch64 0x00010b4878f1 
qemu-system-aarch64 + 346353
29  qemu-system-aarch64 0x00010b4414aa 
qemu-system-aarch64 + 58538
30  qemu-system-aarch64 0x00010b4f78c3 
qemu-system-aarch64 + 805059
31  qemu-system-aarch64 0x00010b487c24 
qemu-system-aarch64 + 347172
32  qemu-system-aarch64 0x00010b487a15 
qemu-system-aarch64 + 346645
33  qemu-system-aarch64 0x00010b4878f1 
qemu-system-aarch64 + 346353
34  qemu-system-aarch64 0x00010b4b8f57 
qemu-system-aarch64 + 548695
35  qemu-system-aarch64 0x00010b49c3af 
qemu-system-aarch64 + 431023
36  ??? 0x0001117891f3 0x0 + 
4588081651
  )
  libc++abi.dylib: terminating with uncaught exception of type NSException
  fish: 'qemu-system-aarch64 -M rasp

Re: [Qemu-devel] [RFC 41/48] configure: add --enable-plugins

2018-11-29 Thread Roman Bolshakov
On Thu, Nov 29, 2018 at 12:49:27PM -0500, Emilio G. Cota wrote:
> On Thu, Nov 29, 2018 at 12:00:55 -0500, Emilio G. Cota wrote:
> > On Thu, Nov 29, 2018 at 12:57:16 +0300, Roman Bolshakov wrote:
> > > Hi Emilio,
> > > 
> > > I think there's an issue with "\s" character class, it's not recognized
> > > by macOS sed  and I'm getting incorrect lines in
> > > qemu-plugins-ld64.symbols:
> > > _  qemu_xxx
> > > _  qemu_xyz
> > > 
> > > After I replaced "\s" with "[[:space:]]", linking proceeds further
> > 
> > Nice, thanks. Will update.
> > 
> > > , but doesn't succeed because of an unresolved reference for qemu-system 
> > > cris,
> > > lm32, m68k, microblaze, microblazeel, moxie, nios2, or1k, riscv32,
> > > riscv64, sparc, unicore32, tricore, xtensa, xtensaeb:
> > > 
> > > Undefined symbols for architecture x86_64:
> > >   "_pci_register_bar", referenced from:
> > >   _plugin_chan_realize in plugin-chan.o
> > > 
> > > It probably has nothing to do with macOS per-se and shouldn't link on
> > > Linux as well. If I disable the aforementioned targets the build
> > > succeeds and I can see the symbols from qemu-plugins-ld64.symbols in
> > > compiled qemu-system binaries.
> > 
> > Yes, that's because plugin-chan should only be built if the guest has PCI
> > support. Will fix.
> 
> Pushed the fixes to the github branch. Hope it works for you now!
> 

Thank you Emilio,
the build succeded with the set of declared symbols exposed in
qemu-system.

I've just noticed qemu-plugins-ld64.symbols should be added to
.gitignore.

Best regards,
Roman



Re: [Qemu-devel] [RFC 48/48] plugin: add a couple of very simple examples

2018-11-29 Thread Roman Bolshakov
On Thu, Oct 25, 2018 at 01:20:57PM -0400, Emilio G. Cota wrote:
> +
> +lib%.so: %.o
> + $(CC) -shared -Wl,-soname,$@ -o $@ $^ $(LDLIBS)

The rule should be a bit different for macOS:
%.bundle: %.o
   $(CC) -bundle -Wl,-bundle_loader,PATH_TO_QEMU_EXE -o $@ $^ $(LDLIBS)

"-bundle" flag is needed because macOS has two kinds of Mach-O
dynamically loaded executables:
 - dylib (MH_DYLIB) - it's like an ELF shared object used for dynamic
   linking. Can be loaded, preloaded for sake of function interposing
   but cannot be explicitly unloaded.
 - bundle (MH_BUNDLE) - similar to an ELF shared object used in plugin
   dlopen/dlclose scenario.

We can pick any (enabled in configure) qemu-system executable as
bundle_loader. We specify it to check if all symbols in a bundle,
including the ones coming from the executable are defined. FWIW, here's
the reference for bundle_loader flag:
  -bundle_loader executable
  This specifies the executable that will be loading the bundle
  output file being linked. Undefined symbols from the bundle are
  checked against the specified executable like it was one of the
  dynamic libraries the bundle was linked with


The ".bundle" extension is not required but IMO it feels more native to
the system than ".so".

I've checked both plugins can be loaded and print a line when QEMU exits.

Best regards,
Roman



[Qemu-devel] [Bug 1802684] Re: QEMU gui crashes on macOS Mojave

2018-11-30 Thread Roman Bolshakov
I've tried to run two x86 guests with Cocoa display on 3.1 rc3, the GUI
doesn't crash. I've tried to change screen resolution on openSUSE 15, it
also works without an issue.

My command line is:
./x86_64-softmmu/qemu-system-x86_64 -accel hvf -cpu host -hda /path/to/disk -m 
MEMORY

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1802684

Title:
  QEMU gui crashes on macOS Mojave

Status in QEMU:
  New

Bug description:
  QEMU release 3.0.0 as well as a recent head build

  /usr/local/Cellar/qemu/HEAD-03c1ca1 (147 files, 257.2MB)
Built from source on 2018-11-06 at 13:41:32 with: --with-gtk+3 --with-sdl2 
--with-libusb
  /usr/local/Cellar/qemu/3.0.0 (137 files, 261.6MB) *
Poured from bottle on 2018-11-10 at 22:58:32 with: --with-gtk+3 
--with-libusb --with-sdl2

  Crashes when attempting to use any gui interface (tried SDL and
  default Cocoa):

  2018-11-10 22:58:41.799 qemu-system-aarch64[42982:1102466] *** Terminating 
app due to uncaught exception 'NSInternalInconsistencyException', reason: 
'NSWindow drag regions should only be invalidated on the Main Thread!'
  *** First throw call stack:
  (
0   CoreFoundation  0x7fff3ea96ecd 
__exceptionPreprocess + 256
1   libobjc.A.dylib 0x7fff6ab49720 
objc_exception_throw + 48
2   CoreFoundation  0x7fff3eab095d 
-[NSException raise] + 9
3   AppKit  0x7fff3bfb13fa 
-[NSWindow(NSWindow_Theme) 
_postWindowNeedsToResetDragMarginsUnlessPostingDisabled] + 324
4   AppKit  0x7fff3bfb6850 -[NSView 
setFrameSize:] + 2082
5   AppKit  0x7fff3c02747d 
-[NSVisualEffectView setFrameSize:] + 171
6   AppKit  0x7fff3c0811b1 
-[NSTitlebarView setFrameSize:] + 84
7   AppKit  0x7fff3bfb5859 -[NSView 
setFrame:] + 478
8   AppKit  0x7fff3c081154 
-[NSTitlebarView resizeWithOldSuperviewSize:] + 100
9   AppKit  0x7fff3bfbc95e -[NSView 
resizeSubviewsWithOldSize:] + 502
10  AppKit  0x7fff3bfb66d9 -[NSView 
setFrameSize:] + 1707
11  AppKit  0x7fff3c9773c0 
-[NSTitlebarContainerView setFrameSize:] + 142
12  AppKit  0x7fff3bfb5859 -[NSView 
setFrame:] + 478
13  AppKit  0x7fff3bfbcdb5 -[NSView 
resizeWithOldSuperviewSize:] + 776
14  AppKit  0x7fff3bfbc95e -[NSView 
resizeSubviewsWithOldSize:] + 502
15  AppKit  0x7fff3bfb66d9 -[NSView 
setFrameSize:] + 1707
16  AppKit  0x7fff3c024570 
-[NSThemeFrame setFrameSize:] + 495
17  AppKit  0x7fff3c011223 -[NSWindow 
_setFrame:updateBorderViewSize:] + 966
18  AppKit  0x7fff3c010b46 -[NSWindow 
_oldPlaceWindow:] + 547
19  AppKit  0x7fff3c010151 -[NSWindow 
_setFrameCommon:display:stashSize:] + 3006
20  AppKit  0x7fff3c00f57d -[NSWindow 
_setFrame:display:allowImplicitAnimation:stashSize:] + 192
21  AppKit  0x7fff3c019ff8 -[NSWindow 
setFrame:display:animate:] + 567
22  qemu-system-aarch64 0x00010b7b2abf 
qemu-system-aarch64 + 3668671
23  qemu-system-aarch64 0x00010b7b6356 
qemu-system-aarch64 + 3683158
24  qemu-system-aarch64 0x00010b7ad836 
qemu-system-aarch64 + 3647542
25  qemu-system-aarch64 0x00010b4ce769 
qemu-system-aarch64 + 636777
26  qemu-system-aarch64 0x00010b487c24 
qemu-system-aarch64 + 347172
27  qemu-system-aarch64 0x00010b487a15 
qemu-system-aarch64 + 346645
28  qemu-system-aarch64 0x00010b4878f1 
qemu-system-aarch64 + 346353
29  qemu-system-aarch64 0x00010b4414aa 
qemu-system-aarch64 + 58538
30  qemu-system-aarch64 0x00010b4f78c3 
qemu-system-aarch64 + 805059
31  qemu-system-aarch64 0x00010b487c24 
qemu-system-aarch64 + 347172
32  qemu-system-aarch64 0x00010b487a15 
qemu-system-aarch64 + 346645
33  qemu-system-aarch64 0x00010b4878f1 
qemu-system-aarch64 + 346353
34  qemu-system-aarch64 0x00010b4b8f57 
qemu-system-aarch64 + 548695
35  qemu-system-aarch64 0x00010b49c3af 
qemu-system-aarch64 + 431023
36  ?

[Qemu-devel] [PATCH for-3.1 2/2] i386: hvf: drop debug printf in decode_sldtgroup

2018-12-03 Thread Roman Bolshakov
It's going to clutter QEMU logs if 0x0f00 is trapped.

Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/x86_decode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
index d125a6ef83..5f513c5563 100644
--- a/target/i386/hvf/x86_decode.c
+++ b/target/i386/hvf/x86_decode.c
@@ -525,8 +525,6 @@ static void decode_sldtgroup(CPUX86State *env, struct 
x86_decode *decode)
 X86_DECODE_CMD_INVL
 };
 decode->cmd = group[decode->modrm.reg];
-printf("%llx: decode_sldtgroup: %d\n", env->hvf_emul->fetch_rip,
-decode->modrm.reg);
 }
 
 static void decode_lidtgroup(CPUX86State *env, struct x86_decode *decode)
-- 
2.17.2 (Apple Git-113)




[Qemu-devel] [PATCH for-3.1 1/2] i386: hvf: Fix overrun of _decode_tbl1

2018-12-03 Thread Roman Bolshakov
Single opcode instructions in ff group were incorrectly processed
because an overrun of _decode_tbl1[0xff] resulted in access of
_decode_tbl2[0x0]. Thus, decode_sldtgroup was called instead of
decode_ffgroup:
  7d71: decode_sldtgroup: 1
  Unimplemented handler (7d71) for 108 (ff 0)

While at it correct maximum length for _decode_tbl2 and _decode_tbl3.

Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/x86_decode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/i386/hvf/x86_decode.c b/target/i386/hvf/x86_decode.c
index 2e33b69541..d125a6ef83 100644
--- a/target/i386/hvf/x86_decode.c
+++ b/target/i386/hvf/x86_decode.c
@@ -454,9 +454,9 @@ struct decode_x87_tbl {
 struct decode_tbl invl_inst = {0x0, 0, 0, false, NULL, NULL, NULL, NULL,
decode_invalid};
 
-struct decode_tbl _decode_tbl1[255];
-struct decode_tbl _decode_tbl2[255];
-struct decode_x87_tbl _decode_tbl3[255];
+struct decode_tbl _decode_tbl1[256];
+struct decode_tbl _decode_tbl2[256];
+struct decode_x87_tbl _decode_tbl3[256];
 
 static void decode_x87_ins(CPUX86State *env, struct x86_decode *decode)
 {
-- 
2.17.2 (Apple Git-113)




[Qemu-devel] [RFC 2/2] qemu-thread: Don't block SEGV, ILL and FPE

2018-12-17 Thread Roman Bolshakov
If any of these signals happen on macOS, they are not delivered to other
threads and signalfd_compat receives nothing. Indeed, POSIX reference
and sigprocmask(2) note that an attempt to block the signals results in
undefined behaviour. SEGV and FPE can't also be received by signalfd(2)
on Linux.

An ability to retrieve SIGBUS via signalfd(2) is used by QEMU for
memory preallocation therefore we can't unblock it without consequences.
But it's important to leave a remark that the signal is lost on macOS.

Signed-off-by: Roman Bolshakov 
---
 util/qemu-thread-posix.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index c6934bd22c..1bf5e65dea 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -524,6 +524,11 @@ void qemu_thread_create(QemuThread *thread, const char 
*name,
 
 /* Leave signal handling to the iothread.  */
 sigfillset(&set);
+/* Blocking the signals can result in undefined behaviour. */
+sigdelset(&set, SIGSEGV);
+sigdelset(&set, SIGFPE);
+sigdelset(&set, SIGILL);
+/* TODO avoid SIGBUS loss on macOS */
 pthread_sigmask(SIG_SETMASK, &set, &oldset);
 
 qemu_thread_args = g_new0(QemuThreadArgs, 1);
-- 
2.19.1




[Qemu-devel] [RFC 1/2] util: Implement debug-threads for macOS

2018-12-17 Thread Roman Bolshakov
macOS provides pthread_setname_np that doesn't have thread id argument.

Signed-off-by: Roman Bolshakov 
---
 configure| 32 ++--
 qemu-options.hx  |  4 ++--
 util/qemu-thread-posix.c |  6 +-
 3 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/configure b/configure
index 224d3071ac..99dc073e53 100755
--- a/configure
+++ b/configure
@@ -3715,8 +3715,8 @@ if test "$mingw32" != yes -a "$pthread" = no; then
   "Make sure to have the pthread libs and headers installed."
 fi
 
-# check for pthread_setname_np
-pthread_setname_np=no
+# check for pthread_setname_np with thread id
+pthread_setname_np_w_tid=no
 cat > $TMPC << EOF
 #include 
 
@@ -3730,7 +3730,24 @@ int main(void)
 }
 EOF
 if compile_prog "" "$pthread_lib" ; then
-  pthread_setname_np=yes
+  pthread_setname_np_w_tid=yes
+fi
+
+# check for pthread_setname_np without thread id
+pthread_setname_np_wo_tid=no
+cat > $TMPC << EOF
+#include 
+
+static void *f(void *p) { pthread_setname_np("QEMU"); }
+int main(void)
+{
+pthread_t thread;
+pthread_create(&thread, 0, f, 0);
+return 0;
+}
+EOF
+if compile_prog "" "$pthread_lib" ; then
+  pthread_setname_np_wo_tid=yes
 fi
 
 ##
@@ -6883,11 +6900,14 @@ fi
 # Hold two types of flag:
 #   CONFIG_THREAD_SETNAME_BYTHREAD  - we've got a way of setting the name on
 # a thread we have a handle to
-#   CONFIG_PTHREAD_SETNAME_NP   - A way of doing it on a particular
+#   CONFIG_PTHREAD_SETNAME_NP_W_TID - A way of doing it on a particular
 # platform
-if test "$pthread_setname_np" = "yes" ; then
+if test "$pthread_setname_np_w_tid" = "yes" ; then
   echo "CONFIG_THREAD_SETNAME_BYTHREAD=y" >> $config_host_mak
-  echo "CONFIG_PTHREAD_SETNAME_NP=y" >> $config_host_mak
+  echo "CONFIG_PTHREAD_SETNAME_NP_W_TID=y" >> $config_host_mak
+elif test "$pthread_setname_np_wo_tid" = "yes" ; then
+  echo "CONFIG_THREAD_SETNAME_BYTHREAD=y" >> $config_host_mak
+  echo "CONFIG_PTHREAD_SETNAME_NP_WO_TID=y" >> $config_host_mak
 fi
 
 if test "$vxhs" = "yes" ; then
diff --git a/qemu-options.hx b/qemu-options.hx
index df42116ecc..d4f3564b78 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -538,8 +538,8 @@ ETEXI
 DEF("name", HAS_ARG, QEMU_OPTION_name,
 "-name string1[,process=string2][,debug-threads=on|off]\n"
 "set the name of the guest\n"
-"string1 sets the window title and string2 the process 
name (on Linux)\n"
-"When debug-threads is enabled, individual threads are 
given a separate name (on Linux)\n"
+"string1 sets the window title and string2 the process 
name\n"
+"When debug-threads is enabled, individual threads are 
given a separate name\n"
 "NOTE: The thread names are for debugging and not a stable 
API.\n",
 QEMU_ARCH_ALL)
 STEXI
diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index 865e476df5..c6934bd22c 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -484,12 +484,16 @@ static void *qemu_thread_start(void *args)
 void *arg = qemu_thread_args->arg;
 void *r;
 
-#ifdef CONFIG_PTHREAD_SETNAME_NP
+#ifdef CONFIG_THREAD_SETNAME_BYTHREAD
 /* Attempt to set the threads name; note that this is for debug, so
  * we're not going to fail if we can't set it.
  */
 if (name_threads && qemu_thread_args->name) {
+# if defined(CONFIG_PTHREAD_SETNAME_NP_W_TID)
 pthread_setname_np(pthread_self(), qemu_thread_args->name);
+# elif defined(CONFIG_PTHREAD_SETNAME_NP_WO_TID)
+pthread_setname_np(qemu_thread_args->name);
+# endif
 }
 #endif
 g_free(qemu_thread_args->name);
-- 
2.19.1




[Qemu-devel] [RFC 0/2] Improve qemu-thread support on macOS

2018-12-17 Thread Roman Bolshakov
Hello,

I've hit a case where QEMU hangs not responding to anything except
SIGKILL. It turned out to be a SIGSEGV in vCPU thread that was lost by
masking all signals.

By blocking too many signals QEMU relies on undefined behaviour that
seems to work on Linux. It's documented in POSIX reference and
sigprocmask(2). Indeed signalfd(2) on Linux notes that it can't be used
to receive SIGSEGV and SIGFPE.

It's not clear what do with SIGBUS on macOS. We can't blindly unblock it
as it's used for memory preallocation.

Also the RFC adds support for thread naming on macOS. Some threads
(signalfd_compat and rcu_call) are created before debug-threads=on is
parsed and don't get their names though.

Thank you,
Roman

Roman Bolshakov (2):
  util: Implement debug-threads for macOS
  qemu-thread: Don't block SEGV, ILL and FPE

 configure| 32 ++--
 qemu-options.hx  |  4 ++--
 util/qemu-thread-posix.c | 11 ++-
 3 files changed, 38 insertions(+), 9 deletions(-)

-- 
2.19.1




Re: [Qemu-devel] [PATCH] ui/cocoa: Include less of the generated modular QAPI headers

2018-12-19 Thread Roman Bolshakov
On Wed, Dec 19, 2018 at 10:12:48AM +0100, Markus Armbruster wrote:
> Avoids pointless recompilation.  Missed in commit 112ed241f5d.
> 
> Signed-off-by: Markus Armbruster 
> ---
> Untested; I don't have access to a Mac.
> 
>  ui/cocoa.m | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index ecf12bfc2e..43e751693c 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -32,7 +32,8 @@
>  #include "ui/input.h"
>  #include "sysemu/sysemu.h"
>  #include "qapi/error.h"
> -#include "qapi/qapi-commands.h"
> +#include "qapi/qapi-commands-block-core.h"
> +#include "qapi/qapi-commands-misc.h"
>  #include "sysemu/blockdev.h"
>  #include "qemu-version.h"
>  #include 
> -- 
> 2.17.2
> 
> 

Hi Markus,

You should change change qapi/qapi-commands-block-core.h to
qapi/qapi-commands-block.h to avoid:

ui/cocoa.m:1225:5: warning: implicit declaration of function 'qmp_eject' is 
invalid in C99 [-Wimplicit-function-declaration]
qmp_eject(true, [drive cStringUsingEncoding: NSASCIIStringEncoding],
^
ui/cocoa.m:1225:5: warning: this function declaration is not a prototype 
[-Wstrict-prototypes]

Thanks,
Roman



Re: [Qemu-devel] [PATCH v2] ui/cocoa: Include less of the generated modular QAPI headers

2018-12-20 Thread Roman Bolshakov
On Thu, Dec 20, 2018 at 09:45:59AM +0100, Markus Armbruster wrote:
> Avoids pointless recompilation.  Missed in commit 112ed241f5d.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  ui/cocoa.m | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Reviewed-by: Roman Bolshakov 
Tested-by: Roman Bolshakov 

Thanks,
Roman



Re: [Qemu-devel] [PATCH] target-i386: hvf: remove MPX support

2019-01-09 Thread Roman Bolshakov
On Thu, Dec 20, 2018 at 01:11:12PM +0100, Paolo Bonzini wrote:
> MPX support is being phased out by Intel and actually I am not sure that
> OS X has ever enabled it in XCR0.  Drop it from the Hypervisor.framework
> acceleration.
> 

I also doubt if OS X enabled it, but I can't confirm that as I have only
Ivy Bridge and Haswell-based laptops.

Reviewed-by: Roman Bolshakov 

Thanks,
Roman



Re: [Qemu-devel] [PATCH] char-pty: remove unnecessary #ifdef

2018-08-30 Thread Roman Bolshakov
On Wed, Aug 22, 2018 at 12:38:35PM +0200, Paolo Bonzini wrote:
> For some reason __APPLE__ was not checked in pty code.  However, the #ifdef
> is redundant: this file is already compiled only if CONFIG_POSIX, same as
> util/qemu-openpty.c which it uses.
> 

Hi Paolo,

Is it possible to apply the patch for master and 3.1 branch?

I'm trying to bring up qemu + libvirt on macOS. If it goes to 3.1, then
libvirt 4.7.0 and QEMU 3.1 are going to be a working solution for
virtualization on macOS.

Lack of pty devices prevents a user from using valid domain xml with
serial console and virsh console command.

--
Thank you,
Roman



[Qemu-devel] [PATCH] char: Enable build of pty on macOS

2018-08-21 Thread Roman Bolshakov
For some reason __APPLE__ was not checked in pty code. pty chardev
should be available on macOS, according to man page.

Signed-off-by: Roman Bolshakov 
---
 chardev/char-pty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index 68fd4e20c3..cb00257ebe 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -33,7 +33,7 @@
 
 #if defined(__linux__) || defined(__sun__) || defined(__FreeBSD__)  \
 || defined(__NetBSD__) || defined(__OpenBSD__) || defined(__DragonFly__) \
-|| defined(__GLIBC__)
+|| defined(__GLIBC__) || defined(__APPLE__)
 
 typedef struct {
 Chardev parent;
-- 
2.15.2 (Apple Git-101.1)




Re: [Qemu-devel] [PATCH] char-pty: remove unnecessary #ifdef

2018-08-22 Thread Roman Bolshakov
On Wed, Aug 22, 2018 at 12:38:35PM +0200, Paolo Bonzini wrote:
> For some reason __APPLE__ was not checked in pty code.  However, the #ifdef
> is redundant: this file is already compiled only if CONFIG_POSIX, same as
> util/qemu-openpty.c which it uses.
> 

Thanks Paolo!

FWIW, qemu_openpty_raw and qemu_pipe are declared for all operating
systems except _WIN32 in qemu-common.h. For sake of symmetry, should the
declarations be available only for CONFIG_POSIX?

--
Roman



[PATCH] i386: hvf: Reset IRQ inhibition after moving RIP

2020-03-28 Thread Roman Bolshakov
The sequence of instructions exposes an issue:
  sti
  hlt

Interrupts cannot be delivered to hvf after hlt instruction cpu because
HF_INHIBIT_IRQ_MASK is set just before hlt is handled and never reset
after moving instruction pointer beyond hlt.

So, after hvf_vcpu_exec() returns, CPU thread gets locked up forever in
qemu_wait_io_event() (cpu_thread_is_idle() evaluates inhibition
flag and considers the CPU idle if the flag is set).

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/vmx.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index 03d2c79b9c..ce2a1532d5 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -167,6 +167,8 @@ static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t 
cr4)
 
 static inline void macvm_set_rip(CPUState *cpu, uint64_t rip)
 {
+X86CPU *x86_cpu = X86_CPU(cpu);
+CPUX86State *env = &x86_cpu->env;
 uint64_t val;
 
 /* BUG, should take considering overlap.. */
@@ -176,6 +178,7 @@ static inline void macvm_set_rip(CPUState *cpu, uint64_t 
rip)
val = rvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY);
if (val & (VMCS_INTERRUPTIBILITY_STI_BLOCKING |
VMCS_INTERRUPTIBILITY_MOVSS_BLOCKING)) {
+env->hflags &= ~HF_INHIBIT_IRQ_MASK;
 wvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY,
val & ~(VMCS_INTERRUPTIBILITY_STI_BLOCKING |
VMCS_INTERRUPTIBILITY_MOVSS_BLOCKING));
-- 
2.24.1




[Bug 1840719] Re: win98se floppy fails to boot with isapc machine

2020-08-13 Thread Roman Bolshakov
** Changed in: qemu
   Status: Fix Committed => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1840719

Title:
  win98se floppy fails to boot with isapc machine

Status in QEMU:
  Fix Released

Bug description:
  QEMU emulator version 4.1.50 (commit 50d69ee0d)

  floppy image from:
  https://winworldpc.com/download/417d71c2-ae18-c39a-11c3-a4e284a2c3a5

  $ qemu-system-i386 -M isapc -fda Windows\ 98\ Second\ Edition\ Boot.img
  SeaBIOS (version rel-1.12.1-0...)
  Booting from Floppy...
  Boot failed: could not read the boot disk

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1840719/+subscriptions



Re: [PATCH v2 000/150] Meson integration for 5.2

2020-08-17 Thread Roman Bolshakov
On Fri, Aug 14, 2020 at 05:10:56AM -0400, Paolo Bonzini wrote:
> News since v1:
> * automatically generate dependencies for sphinx manuals [Peter]
> * fixes for ARM KVM build [Peter]
> * work around old libiscsi in vhost-user-scsi.c [Peter]
> * hack to support default c:/Program Files/QEMU prefix on mingw cross 
> compilation [Peter]
> * added --enable-gettext/--disable-gettext [Peter]
> * test for setuptools presence [Peter]
> * fixes for Darwin [Peter, Roman]
> * do not invoke system Meson through Python, always use embedded Meson with 
> --python [Roman]
> * improvements and fixes to clean and distclean targets [Stefano]
> * avoid (incorrect?) ubsan failure from CONFIG_BDRV_*_WHITELIST [Alexander]
> * move --s390-pgste to Meson, removed QEMU_CFLAGS/QEMU_LDFLAGS for 
> config-target.mak
> * remove some dead configure assignments
> * update build system docs on how to add configure options, convert to rST
> * moved installation of edk2 blobs and descriptors to Meson
> * build and install elf2dmp on Windows too
> * included headers renamed to .c.inc instead of .inc
> 
> Available from https://gitlab.com/bonzini/qemu branch meson-poc-next.
> 
> Paolo
> 

Hi Paolo,

for macOS/darwin,

Tested-by: Roman Bolshakov 

On the next step, it might be good to drop configure in favor of meson
configuration, so configure, build and test commands would be similar to
libvirt:

meson build
ninja -C build
meson test -C build

Thanks,
Roman



Re: [PATCH 025/150] libqemuutil, qapi, trace: convert to meson

2020-08-17 Thread Roman Bolshakov
On Fri, Aug 14, 2020 at 05:11:21AM -0400, Paolo Bonzini wrote:
> This shows how to do some "computations" in meson.build using its array
> and dictionary data structures, and also a basic usage of the sourceset
> module for conditional compilation.
> 
> [...]
> diff --git a/trace/meson.build b/trace/meson.build
> new file mode 100644
> index 00..f0a8d1c2e2
> --- /dev/null
> +++ b/trace/meson.build
> @@ -0,0 +1,76 @@
> +trace_events_files = []
> +foreach dir : [ '.' ] + trace_events_subdirs
> +  trace_events_file = meson.source_root() / dir / 'trace-events'
> +  trace_events_files += [ trace_events_file ]
> +  group_name = dir == '.' ? 'root' : dir.underscorify()
> +  group = '--group=' + group_name
> +  fmt = '@0@-' + group_name + '.@1@'
> +
> +  trace_h = custom_target(fmt.format('trace', 'h'),
> +  output: fmt.format('trace', 'h'),
> +  input: trace_events_file,
> +  command: [ tracetool, group, '--format=h', 
> '@INPUT@' ],
> +  capture: true)
> +  genh += trace_h
> +  trace_c = custom_target(fmt.format('trace', 'c'),
> +  output: fmt.format('trace', 'c'),
> +  input: trace_events_file,
> +  command: [ tracetool, group, '--format=c', 
> '@INPUT@' ],
> +  capture: true)
> +  if 'CONFIG_TRACE_UST' in config_host
> +trace_ust_h = custom_target(fmt.format('trace-ust', 'h'),
> +output: fmt.format('trace-ust', 'h'),
> +input: trace_events_file,
> +command: [ tracetool, group, 
> '--format=ust-events-h', '@INPUT@' ],
> +capture: true)
> +trace_ss.add(trace_ust_h, lttng, urcubp)
> +genh += trace_ust_h
> +  endif
> +  trace_ss.add(trace_h, trace_c)
> +  if 'CONFIG_TRACE_DTRACE' in config_host
> +trace_dtrace = custom_target(fmt.format('trace-dtrace', 'dtrace'),
> + output: fmt.format('trace-dtrace', 
> 'dtrace'),
> + input: trace_events_file,
> + command: [ tracetool, group, '--format=d', 
> '@INPUT@' ],
> + capture: true)
> +trace_dtrace_h = custom_target(fmt.format('trace-dtrace', 'h'),
> +   output: fmt.format('trace-dtrace', 'h'),
> +   input: trace_dtrace,
> +   command: [ 'dtrace', '-o', '@OUTPUT@', 
> '-h', '-s', '@INPUT@' ])
> +trace_dtrace_o = custom_target(fmt.format('trace-dtrace', 'o'),
> +   output: fmt.format('trace-dtrace', 'o'),
> +   input: trace_dtrace,
> +   command: [ 'dtrace', '-o', '@OUTPUT@', 
> '-G', '-s', '@INPUT@' ])
> +
> +trace_ss.add(trace_dtrace_h, trace_dtrace_o)

The patch conflicts with the latest tracing PULL request, object files
shouldn't be generated on darwin:

https://lists.gnu.org/archive/html/qemu-devel/2020-08/msg02493.html

Thanks,
Roman

> +genh += trace_dtrace_h
> +  endif
> +endforeach
> +
> +custom_target('trace-events-all',
> +  output: 'trace-events-all',
> +  input: trace_events_files,
> +  command: [ 'cat', '@INPUT@' ],
> +  capture: true,
> +  install: true,
> +  install_dir: config_host['qemu_datadir'])
> +
> +if 'CONFIG_TRACE_UST' in config_host
> +  trace_ust_all_h = custom_target('trace-ust-all.h',
> +  output: 'trace-ust-all.h',
> +  input: trace_events_files,
> +  command: [ tracetool, '--group=all', 
> '--format=ust-events-h', '@INPUT@' ],
> +  capture: true)
> +  trace_ust_all_c = custom_target('trace-ust-all.c',
> +  output: 'trace-ust-all.c',
> +  input: trace_events_files,
> +  command: [ tracetool, '--group=all', 
> '--format=ust-events-c', '@INPUT@' ],
> +  capture: true)
> +  trace_ss.add(trace_ust_all_h, trace_ust_all_c)
> +  genh += trace_ust_all_h
> +endif
> +
> +trace_ss.add(when: 'CONFIG_TRACE_SIMPLE', if_true: files('simple.c'))
> +trace_ss.add(when: 'CONFIG_TRACE_FTRACE', if_true: files('ftrace.c'))
> +trace_ss.add(files('control.c'))
> +trace_ss.add(files('qmp.c'))



Re: [PATCH v2 000/150] Meson integration for 5.2

2020-08-17 Thread Roman Bolshakov
On Mon, Aug 17, 2020 at 01:24:50PM +0200, Paolo Bonzini wrote:
> On 17/08/20 13:02, Roman Bolshakov wrote:
> > 
> > Tested-by: Roman Bolshakov 
> > 
> > On the next step, it might be good to drop configure in favor of meson
> > configuration, so configure, build and test commands would be similar to
> > libvirt:
> > 
> > meson build
> > ninja -C build
> > meson test -C build
> 
> Well, there are quite a few steps needed to get there:
> 
> 1) moving feature tests from configure to Meson is a no-brainer.
> However it's better to first convert the unit tests to Meson and get rid
> of the rest of rules.mak.  This is because it's simpler to do this if
> there are no uses left of CONFIG_* symbols in the Makefiles.
> 

Agreed

> 2) moving the rest of "make install" to Meson is even more of a
> no-brainer.  Marc-André has patches there.  They also include a
> conversion of the ROM build.
> 
> 3) moving the bulk of the build from meson to ninja is possible and I
> already have patches for it.  It requires adding ninja as a build
> dependency however.  The main advantage here is getting rid of
> Ninjatool.  I'm not sure how moving the handling of submodules to Meson
> would work, and that's needed in order to be able to build with "ninja
> -C build".
> 

I'm more familiar with CMake rather than meson but they seem to be
similar. CMake has a way to wrap build of submodules:
https://cmake.org/cmake/help/latest/module/ExternalProject.html

I haven't found if meson has the feature. Perhaps, like 4) it's
something to be enhanced in meson.

An alternative is to change the submodules to Meson (if their
maintainers don't mind the change) and use meson subproject:

https://mesonbuild.com/Subprojects.html

Regards,
Roman

> 4) I find "meson test" to be inferior in some respects to the QEMU's TAP
> test harness.  In particular, one feature of QEMU's Makefiles is that
> you can cut-and-paste from "make V=1 output" into the shell.  So that
> part may take some time.  I'd rather extend Meson so that it's possible
> to write arbitrary test runners.
> 



Re: [PATCH v5 10/14] cpus: add handle_interrupt to the CpusAccel interface

2020-08-17 Thread Roman Bolshakov
On Fri, Aug 14, 2020 at 02:01:01PM -0700, Richard Henderson wrote:
> On 8/12/20 11:32 AM, Claudio Fontana wrote:
> > +static void generic_handle_interrupt(CPUState *cpu, int mask)
> > +{
> > +cpu->interrupt_request |= mask;
> > +
> > +if (!qemu_cpu_is_self(cpu)) {
> > +qemu_cpu_kick(cpu);
> > +}
> > +}
> > +
> > +void cpu_interrupt(CPUState *cpu, int mask)
> > +{
> > +if (cpus_accel && cpus_accel->handle_interrupt) {
> > +cpus_accel->handle_interrupt(cpu, mask);
> > +} else {
> > +generic_handle_interrupt(cpu, mask);
> > +}
> > +}
> 
> First, by this point you have converted all of the accelerators, so I would
> expect cpus_accel to always be non-null.  I would expect a patch immediately
> preceding this one to place an assert to that effect somewhere in the startup
> code, and to remove all of the checks.
> 
> Second, I would prefer that all methods be non-null, so that you don't need to
> check that either.  This patch would add generic_handle_interrupt (perhaps
> named cpus_accel_default_handle_interrupt declared in sysemu/cpus.h?) to the
> CpusAccel structure of all except TCG.
> 
> Similarly for all other methods that are checking non-null-ness of the method
> pointer.  Perhaps assert non-null for each method in cpus_register_accel().
> 
> 

I concur with that. It's similar to my comment to the previous revision
of the series.

Regards,
Roman



Re: [PATCH 00/41] qom: Automated conversion of type checking boilerplate

2020-08-18 Thread Roman Bolshakov
On Thu, Aug 13, 2020 at 06:25:44PM -0400, Eduardo Habkost wrote:
> This is an extension of the series previously submitted by
> Daniel[1], including a script that will convert existing type
> checker macros automatically.
> 

Hi Eduardo,

do you have a repo where it can be checked it out?

Thanks,
Roman



Re: [PATCH 13/41] hvf: Add missing include

2020-08-18 Thread Roman Bolshakov
On Thu, Aug 13, 2020 at 06:25:57PM -0400, Eduardo Habkost wrote:
> The sysemu/accel.h header is needed for the ACCEL_CLASS_NAME
> macro.  This will be necessary to allow us to use OBJECT_DEFINE*()
> for TYPE_HVF_ACCEL.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/sysemu/hvf.h | 2 ++
>  1 file changed, 2 insertions(+)
> 

Reviewed-by: Roman Bolshakov 



Re: Suspicious QOM types without instance/class size

2020-08-21 Thread Roman Bolshakov
On Thu, Aug 20, 2020 at 05:55:29PM -0400, Eduardo Habkost wrote:
> While trying to convert TypeInfo declarations to the new
> OBJECT_DECLARE* macros, I've stumbled on a few suspicious cases
> where instance_size or class_size is not set, despite having type
> checker macros that use a specific type.
> 
> The ones with "WARNING" are abstract types (maybe not serious if
> subclasses set the appropriate sizes).  The ones with "ERROR"
> don't seem to be abstract types.
> 

> ERROR: target/i386/hvf/hvf.c:908:1: instance_size should be set to 
> sizeof(HVFState)?

Hi Eduardo,

How do you get the error?

Given your changes, instance size should really be sizeof(HVFState).

BTW, the object definition for hvf seems different from KVM (and perhaps
wrong?), e.g. HVFState is allocated within init_machine handler and then
assigned to a global variable:

static int hvf_accel_init(MachineState *ms)
{
int x;
hv_return_t ret;
HVFState *s;

ret = hv_vm_create(HV_VM_DEFAULT);
assert_hvf_ok(ret);

s = g_new0(HVFState, 1);
 
s->num_slots = 32;
for (x = 0; x < s->num_slots; ++x) {
s->slots[x].size = 0;
s->slots[x].slot_id = x;
}
  
hvf_state = s;
cpu_interrupt_handler = hvf_handle_interrupt;
memory_listener_register(&hvf_memory_listener, &address_space_memory);
return 0;
}

static void hvf_accel_class_init(ObjectClass *oc, void *data)
{
AccelClass *ac = ACCEL_CLASS(oc);
ac->name = "HVF";
ac->init_machine = hvf_accel_init;
ac->allowed = &hvf_allowed;
}

static const TypeInfo hvf_accel_type = {
.name = TYPE_HVF_ACCEL,
.parent = TYPE_ACCEL,
.class_init = hvf_accel_class_init,
};

Thanks,
Roman



Re: [PATCH v2 19/58] hvf: Move HVFState typedef to hvf.h

2020-08-21 Thread Roman Bolshakov
On Wed, Aug 19, 2020 at 08:11:57PM -0400, Eduardo Habkost wrote:
> Move typedef closer to the type check macros, to make it easier
> to convert the code to OBJECT_DEFINE_TYPE() in the future.
> 
> Reviewed-by: Daniel P. Berrangé 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2: none
> 
> ---
> Cc: Cameron Esfahani 
> Cc: Roman Bolshakov 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: qemu-devel@nongnu.org
> ---
>  include/sysemu/hvf.h   | 1 +
>  target/i386/hvf/hvf-i386.h | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> index d3bed80ea8..760d6c79a2 100644
> --- a/include/sysemu/hvf.h
> +++ b/include/sysemu/hvf.h
> @@ -35,6 +35,7 @@ void hvf_vcpu_destroy(CPUState *);
>  
>  #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
>  
> +typedef struct HVFState HVFState;
>  #define HVF_STATE(obj) \
>  OBJECT_CHECK(HVFState, (obj), TYPE_HVF_ACCEL)
>  
> diff --git a/target/i386/hvf/hvf-i386.h b/target/i386/hvf/hvf-i386.h
> index ef20c73eca..e0edffd077 100644
> --- a/target/i386/hvf/hvf-i386.h
> +++ b/target/i386/hvf/hvf-i386.h
> @@ -57,13 +57,13 @@ typedef struct hvf_vcpu_caps {
>  uint64_t vmx_cap_preemption_timer;
>  } hvf_vcpu_caps;
>  
> -typedef struct HVFState {
> +struct HVFState {
>  AccelState parent;
>  hvf_slot slots[32];
>  int num_slots;
>  
>  hvf_vcpu_caps *hvf_caps;
> -} HVFState;
> +};
>  extern HVFState *hvf_state;
>  
>  void hvf_set_phys_mem(MemoryRegionSection *, bool);
> -- 
> 2.26.2
> 

Reviewed-by: Roman Bolshakov 

But it seems the accel misuses QOM as pointed out in the other thread.
I also don't see a place where HVF_STATE is invoked... ah, you're
replacing it later in "Use DECLARE_*CHECKER* macros".

Thanks,
Roman



Re: [PATCH v2 19/58] hvf: Move HVFState typedef to hvf.h

2020-08-21 Thread Roman Bolshakov
On Wed, Aug 19, 2020 at 08:11:57PM -0400, Eduardo Habkost wrote:
> Move typedef closer to the type check macros, to make it easier
> to convert the code to OBJECT_DEFINE_TYPE() in the future.
> 
> Reviewed-by: Daniel P. Berrangé 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v1 -> v2: none
> 
> ---
> Cc: Cameron Esfahani 
> Cc: Roman Bolshakov 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: qemu-devel@nongnu.org
> ---
>  include/sysemu/hvf.h   | 1 +
>  target/i386/hvf/hvf-i386.h | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
> index d3bed80ea8..760d6c79a2 100644
> --- a/include/sysemu/hvf.h
> +++ b/include/sysemu/hvf.h
> @@ -35,6 +35,7 @@ void hvf_vcpu_destroy(CPUState *);
>  
>  #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
>  
> +typedef struct HVFState HVFState;

Forward declaration of HVFState is missed before the typedef.

>  #define HVF_STATE(obj) \
>  OBJECT_CHECK(HVFState, (obj), TYPE_HVF_ACCEL)
>  
> diff --git a/target/i386/hvf/hvf-i386.h b/target/i386/hvf/hvf-i386.h
> index ef20c73eca..e0edffd077 100644
> --- a/target/i386/hvf/hvf-i386.h
> +++ b/target/i386/hvf/hvf-i386.h
> @@ -57,13 +57,13 @@ typedef struct hvf_vcpu_caps {
>  uint64_t vmx_cap_preemption_timer;
>  } hvf_vcpu_caps;
>  
> -typedef struct HVFState {
> +struct HVFState {
>  AccelState parent;
>  hvf_slot slots[32];
>  int num_slots;
>  
>  hvf_vcpu_caps *hvf_caps;
> -} HVFState;
> +};
>  extern HVFState *hvf_state;
>  
>  void hvf_set_phys_mem(MemoryRegionSection *, bool);
> -- 
> 2.26.2
> 



Re: [PATCH] configure: Don't warn about lack of PIE on macOS

2020-06-23 Thread Roman Bolshakov
On Tue, Jun 23, 2020 at 01:48:57PM +0100, Stefan Hajnoczi wrote:
> On Mon, Jun 01, 2020 at 03:42:57PM +0300, Roman Bolshakov wrote:
> > ld64 is making PIE executables for 10.7 and above by default, as
> > documented in ld(1).
> > 
> > Signed-off-by: Roman Bolshakov 
> > ---
> >  configure | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/configure b/configure
> > index af2ba83f0e..6dddbca4b2 100755
> > --- a/configure
> > +++ b/configure
> > @@ -2137,6 +2137,8 @@ elif compile_prog "-Werror -fPIE -DPIE" "-pie"; then
> >QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> >QEMU_LDFLAGS="-pie $QEMU_LDFLAGS"
> >pie="yes"
> > +elif test "$darwin" = "yes"; then
> > +  pie="yes"
> 
> Hi Roman,
> I'm wondering why the elif above doesn't detect the presence of PIE
> automatically?
> 
>   elif compile_prog "-Werror -fPIE -DPIE" "-pie"; then
> QEMU_CFLAGS="-fPIE -DPIE $QEMU_CFLAGS"
> QEMU_LDFLAGS="-pie $QEMU_LDFLAGS"
> pie="yes"
> 
> Can this code be tweaked to cover macOS too?
> 
> Also CCing Peter Maydell in case he wants to merge this patch directly
> into qemu.git.
> 
> Stefan

Hi Stefan,

It's because clang does not accept -pie/-no-pie directly:
  $ cc -Werror -fPIE -DPIE -pie main.c
  clang: error: argument unused during compilation: '-pie' 
[-Werror,-Wunused-command-line-argument]

It has to be passed as linker option, i.e. -Wl,-pie or -Wl,-no_pie. pie
is also a default behaviour of clang/ld64.

I had a patch to enable proper support of pie/no-pie for macOS but I see
little value in it. I don't know where no_pie would be helfpul becuase
clang from Apple Developer Tools can't cross-compile option ROMs.

Thanks,
Roman



Re: [RFC RESEND v7 0/4] QEMU cpus.c refactoring

2020-06-24 Thread Roman Bolshakov
On Mon, Jun 22, 2020 at 03:45:30PM +0200, Claudio Fontana wrote:
> Motivation and higher level steps:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg04628.html
> 
> MAIN OPEN POINTS:
> 
> * confirmation on hvf state (Roman).
> 

Hi Claudio,

I'm sorry for delay. I'm wrapping up another round of HVF cleanup that
should make it easier to do hvf-specific changes in patch 4. I expect to
send the cleanup series today.

> * missing reviewed-by for patch 4 in the series
> 

I wonder if it would help to split patch 4 out of the series if nobody
objects patches 1-3. At least it will be easier to rebase the last one
alone. Then patch 4 might become a series on its own.

Regards,
Roman



[PATCH 1/8] i386: hvf: Set env->eip in macvm_set_rip()

2020-06-24 Thread Roman Bolshakov
cpu_synchronize_state() is currently no-op for hvf but BIOS will hang in
vAPIC option ROM when cpu_synchronize_state() is wired to
hvf_cpu_synchronize_state().

cpu_synchronize_state() state is called from vapic_write() during option
ROM initialization. It sets dirty flag on the cpu. macvm_set_rip() is
then invoked to advance IP after the I/O write to vAPIC port.

macvm_set_rip() only modifies VMCS, it doesn't change env->eip.
Therefore on the next iteration of vCPU loop, vcpu_dirty flag is checked
and hvf_put_registers() overwrites correct RIP in VMCS with the value of
env->eip that points to the I/O write instruction. Execution of the CPU
gets stuck on the instruction.

The issue can be avoided if eip doesn't contain stale value when dirty
flag is set on cpu.

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/vmx.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index ce2a1532d5..1e8b29bf7d 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -173,6 +173,7 @@ static inline void macvm_set_rip(CPUState *cpu, uint64_t 
rip)
 
 /* BUG, should take considering overlap.. */
 wreg(cpu->hvf_fd, HV_X86_RIP, rip);
+env->eip = rip;
 
 /* after moving forward in rip, we need to clean INTERRUPTABILITY */
val = rvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY);
-- 
2.26.1




[PATCH 3/8] i386: hvf: Add hvf_cpu_synchronize_pre_loadvm()

2020-06-24 Thread Roman Bolshakov
hvf lacks an implementation of cpu_synchronize_pre_loadvm().

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 include/sysemu/hvf.h  |  1 +
 include/sysemu/hw_accel.h |  3 +++
 target/i386/hvf/hvf.c | 11 +++
 3 files changed, 15 insertions(+)

diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index 5214ed5202..1d40a8ec01 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -28,6 +28,7 @@ int hvf_vcpu_exec(CPUState *);
 void hvf_cpu_synchronize_state(CPUState *);
 void hvf_cpu_synchronize_post_reset(CPUState *);
 void hvf_cpu_synchronize_post_init(CPUState *);
+void hvf_cpu_synchronize_pre_loadvm(CPUState *);
 void hvf_vcpu_destroy(CPUState *);
 void hvf_reset_vcpu(CPUState *);
 
diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
index 80bce75921..e128f8b06b 100644
--- a/include/sysemu/hw_accel.h
+++ b/include/sysemu/hw_accel.h
@@ -73,6 +73,9 @@ static inline void cpu_synchronize_pre_loadvm(CPUState *cpu)
 if (hax_enabled()) {
 hax_cpu_synchronize_pre_loadvm(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_pre_loadvm(cpu);
+}
 if (whpx_enabled()) {
 whpx_cpu_synchronize_pre_loadvm(cpu);
 }
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index be016b951a..efe9802962 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -325,6 +325,17 @@ void hvf_cpu_synchronize_post_init(CPUState *cpu_state)
 run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
 }
 
+static void do_hvf_cpu_synchronize_pre_loadvm(CPUState *cpu,
+  run_on_cpu_data arg)
+{
+cpu->vcpu_dirty = true;
+}
+
+void hvf_cpu_synchronize_pre_loadvm(CPUState *cpu)
+{
+run_on_cpu(cpu, do_hvf_cpu_synchronize_pre_loadvm, RUN_ON_CPU_NULL);
+}
+
 static bool ept_emulation_fault(hvf_slot *slot, uint64_t gpa, uint64_t 
ept_qual)
 {
 int read, write;
-- 
2.26.1




[PATCH 0/8] Improve synchronization between QEMU and HVF

2020-06-24 Thread Roman Bolshakov
The series is a prerequisite to implement gdbstub support for HVF and mostly
concerns improvements of cpu_synchronize_* functions wrt to HVF and addresses
old TODO's in the related code.

Unfortunately live snapshots don't seem to work yet but they don't work with
tcg (on macOS) either.

Roman Bolshakov (8):
  i386: hvf: Set env->eip in macvm_set_rip()
  i386: hvf: Move synchronize functions to sysemu
  i386: hvf: Add hvf_cpu_synchronize_pre_loadvm()
  i386: hvf: Implement CPU kick
  i386: hvf: Don't duplicate register reset
  i386: hvf: Drop hvf_reset_vcpu()
  i386: hvf: Clean up synchronize functions
  MAINTAINERS: Add Cameron as HVF co-maintainer

 MAINTAINERS   |   2 +
 cpus.c|  25 +++---
 include/hw/core/cpu.h |   2 +-
 include/sysemu/hvf.h  |   3 +-
 include/sysemu/hw_accel.h |  13 
 target/i386/cpu.c |   3 -
 target/i386/hvf/hvf.c | 159 --
 target/i386/hvf/vmx.h |   1 +
 8 files changed, 77 insertions(+), 131 deletions(-)

-- 
2.26.1




[PATCH 7/8] i386: hvf: Clean up synchronize functions

2020-06-24 Thread Roman Bolshakov
Make them more concise and consitent with the rest of the code in the
file and drop non-relevant TODO.

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/hvf.c | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 0b2be8de47..18fe843834 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -282,27 +282,24 @@ void hvf_handle_io(CPUArchState *env, uint16_t port, void 
*buffer,
 }
 }
 
-/* TODO: synchronize vcpu state */
 static void do_hvf_cpu_synchronize_state(CPUState *cpu, run_on_cpu_data arg)
 {
-CPUState *cpu_state = cpu;
-if (cpu_state->vcpu_dirty == 0) {
-hvf_get_registers(cpu_state);
+if (!cpu->vcpu_dirty) {
+hvf_get_registers(cpu);
+cpu->vcpu_dirty = true;
 }
-
-cpu_state->vcpu_dirty = 1;
 }
 
-void hvf_cpu_synchronize_state(CPUState *cpu_state)
+void hvf_cpu_synchronize_state(CPUState *cpu)
 {
-if (cpu_state->vcpu_dirty == 0) {
-run_on_cpu(cpu_state, do_hvf_cpu_synchronize_state, RUN_ON_CPU_NULL);
+if (!cpu->vcpu_dirty) {
+run_on_cpu(cpu, do_hvf_cpu_synchronize_state, RUN_ON_CPU_NULL);
 }
 }
 
-static void do_hvf_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data 
arg)
+static void do_hvf_cpu_synchronize_post_reset(CPUState *cpu,
+  run_on_cpu_data arg)
 {
-CPUState *cpu_state = cpu;
 uint64_t pdpte[4] = {0, 0, 0, 0};
 int i;
 
@@ -314,26 +311,25 @@ static void do_hvf_cpu_synchronize_post_reset(CPUState 
*cpu, run_on_cpu_data arg
 wvmcs(cpu->hvf_fd, VMCS_GUEST_PDPTE0 + i * 2, pdpte[i]);
 }
 
-hvf_put_registers(cpu_state);
-cpu_state->vcpu_dirty = false;
+hvf_put_registers(cpu);
+cpu->vcpu_dirty = false;
 }
 
-void hvf_cpu_synchronize_post_reset(CPUState *cpu_state)
+void hvf_cpu_synchronize_post_reset(CPUState *cpu)
 {
-run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_reset, RUN_ON_CPU_NULL);
+run_on_cpu(cpu, do_hvf_cpu_synchronize_post_reset, RUN_ON_CPU_NULL);
 }
 
 static void do_hvf_cpu_synchronize_post_init(CPUState *cpu,
  run_on_cpu_data arg)
 {
-CPUState *cpu_state = cpu;
-hvf_put_registers(cpu_state);
-cpu_state->vcpu_dirty = false;
+hvf_put_registers(cpu);
+cpu->vcpu_dirty = false;
 }
 
-void hvf_cpu_synchronize_post_init(CPUState *cpu_state)
+void hvf_cpu_synchronize_post_init(CPUState *cpu)
 {
-run_on_cpu(cpu_state, do_hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
+run_on_cpu(cpu, do_hvf_cpu_synchronize_post_init, RUN_ON_CPU_NULL);
 }
 
 static void do_hvf_cpu_synchronize_pre_loadvm(CPUState *cpu,
-- 
2.26.1




[PATCH 2/8] i386: hvf: Move synchronize functions to sysemu

2020-06-24 Thread Roman Bolshakov
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 cpus.c| 12 
 include/sysemu/hw_accel.h | 10 ++
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/cpus.c b/cpus.c
index 7317ae06b9..26709677d3 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1017,10 +1017,6 @@ void cpu_synchronize_all_states(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_state(cpu);
-/* TODO: move to cpu_synchronize_state() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_state(cpu);
-}
 }
 }
 
@@ -1030,10 +1026,6 @@ void cpu_synchronize_all_post_reset(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_post_reset(cpu);
-/* TODO: move to cpu_synchronize_post_reset() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_post_reset(cpu);
-}
 }
 }
 
@@ -1043,10 +1035,6 @@ void cpu_synchronize_all_post_init(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_post_init(cpu);
-/* TODO: move to cpu_synchronize_post_init() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_post_init(cpu);
-}
 }
 }
 
diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
index 0ec2372477..80bce75921 100644
--- a/include/sysemu/hw_accel.h
+++ b/include/sysemu/hw_accel.h
@@ -14,6 +14,7 @@
 #include "hw/core/cpu.h"
 #include "sysemu/hax.h"
 #include "sysemu/kvm.h"
+#include "sysemu/hvf.h"
 #include "sysemu/whpx.h"
 
 static inline void cpu_synchronize_state(CPUState *cpu)
@@ -24,6 +25,9 @@ static inline void cpu_synchronize_state(CPUState *cpu)
 if (hax_enabled()) {
 hax_cpu_synchronize_state(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_state(cpu);
+}
 if (whpx_enabled()) {
 whpx_cpu_synchronize_state(cpu);
 }
@@ -37,6 +41,9 @@ static inline void cpu_synchronize_post_reset(CPUState *cpu)
 if (hax_enabled()) {
 hax_cpu_synchronize_post_reset(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_post_reset(cpu);
+}
 if (whpx_enabled()) {
 whpx_cpu_synchronize_post_reset(cpu);
 }
@@ -50,6 +57,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu)
 if (hax_enabled()) {
 hax_cpu_synchronize_post_init(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_post_init(cpu);
+}
 if (whpx_enabled()) {
 whpx_cpu_synchronize_post_init(cpu);
 }
-- 
2.26.1




[PATCH 5/8] i386: hvf: Don't duplicate register reset

2020-06-24 Thread Roman Bolshakov
hvf_reset_vcpu() duplicates actions performed by x86_cpu_reset(). The
difference is that hvf_reset_vcpu() stores initial values directly to
VMCS while x86_cpu_reset() stores it in CPUX86State and then
cpu_synchronize_all_post_init() or cpu_synchronize_all_post_reset()
flushes CPUX86State into VMCS. That makes most of the hvf_reset_vcpu() a
kind of no-op.

Here's the trace of CPU state modifications during VM start:
  hvf_reset_vcpu (resets VMCS)
  cpu_synchronize_all_post_init (overwrites VMCS fields written by
 hvf_reset_vcpu())
  cpu_synchronize_all_states
  hvf_reset_vcpu (resets VMCS)
  cpu_synchronize_all_post_reset (overwrites VMCS fields written by
  hvf_reset_vcpu())

General purpose registers, system registers, segment descriptors, flags
and IP are set by hvf_put_segments() in post-init and post-reset,
therefore it's safe to remove them from hvf_reset_vcpu().

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/hvf.c | 78 ---
 1 file changed, 78 deletions(-)

diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 4d254a477a..2c4028d08c 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -456,90 +456,12 @@ void hvf_reset_vcpu(CPUState *cpu) {
 uint64_t pdpte[4] = {0, 0, 0, 0};
 int i;
 
-/* TODO: this shouldn't be needed; there is already a call to
- * cpu_synchronize_all_post_reset in vl.c
- */
 wvmcs(cpu->hvf_fd, VMCS_ENTRY_CTLS, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_IA32_EFER, 0);
 
 /* Initialize PDPTE */
 for (i = 0; i < 4; i++) {
 wvmcs(cpu->hvf_fd, VMCS_GUEST_PDPTE0 + i * 2, pdpte[i]);
 }
-
-macvm_set_cr0(cpu->hvf_fd, 0x6010);
-
-wvmcs(cpu->hvf_fd, VMCS_CR4_MASK, CR4_VMXE_MASK);
-wvmcs(cpu->hvf_fd, VMCS_CR4_SHADOW, 0x0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_CR4, CR4_VMXE_MASK);
-
-/* set VMCS guest state fields */
-wvmcs(cpu->hvf_fd, VMCS_GUEST_CS_SELECTOR, 0xf000);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_CS_LIMIT, 0x);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_CS_ACCESS_RIGHTS, 0x9b);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_CS_BASE, 0x);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_DS_SELECTOR, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_DS_LIMIT, 0x);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_DS_ACCESS_RIGHTS, 0x93);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_DS_BASE, 0);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_ES_SELECTOR, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_ES_LIMIT, 0x);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_ES_ACCESS_RIGHTS, 0x93);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_ES_BASE, 0);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_FS_SELECTOR, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_FS_LIMIT, 0x);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_FS_ACCESS_RIGHTS, 0x93);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_FS_BASE, 0);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_GS_SELECTOR, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_GS_LIMIT, 0x);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_GS_ACCESS_RIGHTS, 0x93);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_GS_BASE, 0);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_SS_SELECTOR, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_SS_LIMIT, 0x);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_SS_ACCESS_RIGHTS, 0x93);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_SS_BASE, 0);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_LDTR_SELECTOR, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_LDTR_LIMIT, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_LDTR_ACCESS_RIGHTS, 0x1);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_LDTR_BASE, 0);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_TR_SELECTOR, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_TR_LIMIT, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_TR_ACCESS_RIGHTS, 0x83);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_TR_BASE, 0);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_GDTR_LIMIT, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_GDTR_BASE, 0);
-
-wvmcs(cpu->hvf_fd, VMCS_GUEST_IDTR_LIMIT, 0);
-wvmcs(cpu->hvf_fd, VMCS_GUEST_IDTR_BASE, 0);
-
-/*wvmcs(cpu->hvf_fd, VMCS_GUEST_CR2, 0x0);*/
-wvmcs(cpu->hvf_fd, VMCS_GUEST_CR3, 0x0);
-
-wreg(cpu->hvf_fd, HV_X86_RIP, 0xfff0);
-wreg(cpu->hvf_fd, HV_X86_RDX, 0x623);
-wreg(cpu->hvf_fd, HV_X86_RFLAGS, 0x2);
-wreg(cpu->hvf_fd, HV_X86_RSP, 0x0);
-wreg(cpu->hvf_fd, HV_X86_RAX, 0x0);
-wreg(cpu->hvf_fd, HV_X86_RBX, 0x0);
-wreg(cpu->hvf_fd, HV_X86_RCX, 0x0);
-wreg(cpu->hvf_fd, HV_X86_RSI, 0x0);
-wreg(cpu->hvf_fd, HV_X86_RDI, 0x0);
-wreg(cpu->hvf_fd, HV_X86_RBP, 0x0);
-
-for (int i = 0; i < 8; i++) {
-wreg(cpu->hvf_fd, HV_X86_R8 + i, 0x0);
-}
-
-hv_vcpu_invalidate_tlb(cpu->hvf_fd);
-hv_vcpu_flush(cpu->hvf_fd);
 }
 
 void hvf_vcpu_destroy(CPUState *cpu)
-- 
2.26.1




[PATCH 6/8] i386: hvf: Drop hvf_reset_vcpu()

2020-06-24 Thread Roman Bolshakov
It's worth to have a custom accel-specific reset in x86_cpu_reset() only
if something related to CPUState has to be reset and that can't be done
in post-init or post-reset.

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 include/sysemu/hvf.h  |  1 -
 target/i386/cpu.c |  3 ---
 target/i386/hvf/hvf.c | 23 +++
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index aaa00cbf05..a1ab61403f 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -31,7 +31,6 @@ void hvf_cpu_synchronize_post_reset(CPUState *);
 void hvf_cpu_synchronize_post_init(CPUState *);
 void hvf_cpu_synchronize_pre_loadvm(CPUState *);
 void hvf_vcpu_destroy(CPUState *);
-void hvf_reset_vcpu(CPUState *);
 
 #define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf")
 
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b1b311baa2..bfa3eed9b6 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6089,9 +6089,6 @@ static void x86_cpu_reset(DeviceState *dev)
 if (kvm_enabled()) {
 kvm_arch_reset_vcpu(cpu);
 }
-else if (hvf_enabled()) {
-hvf_reset_vcpu(s);
-}
 #endif
 }
 
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 2c4028d08c..0b2be8de47 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -303,6 +303,17 @@ void hvf_cpu_synchronize_state(CPUState *cpu_state)
 static void do_hvf_cpu_synchronize_post_reset(CPUState *cpu, run_on_cpu_data 
arg)
 {
 CPUState *cpu_state = cpu;
+uint64_t pdpte[4] = {0, 0, 0, 0};
+int i;
+
+/* Reset IA-32e mode guest (LMA) */
+wvmcs(cpu->hvf_fd, VMCS_ENTRY_CTLS, 0);
+
+/* Initialize PDPTE */
+for (i = 0; i < 4; i++) {
+wvmcs(cpu->hvf_fd, VMCS_GUEST_PDPTE0 + i * 2, pdpte[i]);
+}
+
 hvf_put_registers(cpu_state);
 cpu_state->vcpu_dirty = false;
 }
@@ -452,18 +463,6 @@ static MemoryListener hvf_memory_listener = {
 .log_sync = hvf_log_sync,
 };
 
-void hvf_reset_vcpu(CPUState *cpu) {
-uint64_t pdpte[4] = {0, 0, 0, 0};
-int i;
-
-wvmcs(cpu->hvf_fd, VMCS_ENTRY_CTLS, 0);
-
-/* Initialize PDPTE */
-for (i = 0; i < 4; i++) {
-wvmcs(cpu->hvf_fd, VMCS_GUEST_PDPTE0 + i * 2, pdpte[i]);
-}
-}
-
 void hvf_vcpu_destroy(CPUState *cpu)
 {
 X86CPU *x86_cpu = X86_CPU(cpu);
-- 
2.26.1




[PATCH 4/8] i386: hvf: Implement CPU kick

2020-06-24 Thread Roman Bolshakov
HVF doesn't have a CPU kick and without it it's not possible to perform
an action on CPU thread until a VMEXIT happens. The kick is also needed
for timely interrupt delivery.

Existing implementation of CPU kick sends SIG_IPI (aka SIGUSR1) to vCPU
thread, but it's different from what hv_vcpu_interrupt does. The latter
one results in invocation of mp_cpus_kick() in XNU kernel [1].

While at it, correct type of hvf_fd to the type of hv_vcpuid_t to avoid
compilation warnings.

1. https://opensource.apple.com/source/xnu/xnu-6153.81.5/osfmk/i386/mp.c

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 cpus.c| 13 +
 include/hw/core/cpu.h |  2 +-
 include/sysemu/hvf.h  |  1 +
 target/i386/hvf/hvf.c | 11 +++
 4 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/cpus.c b/cpus.c
index 26709677d3..36f38ce5c8 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1783,10 +1783,15 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
 return;
 }
 cpu->thread_kicked = true;
-err = pthread_kill(cpu->thread->thread, SIG_IPI);
-if (err && err != ESRCH) {
-fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
-exit(1);
+
+if (hvf_enabled()) {
+hvf_vcpu_kick(cpu);
+} else {
+err = pthread_kill(cpu->thread->thread, SIG_IPI);
+if (err && err != ESRCH) {
+fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
+exit(1);
+}
 }
 #else /* _WIN32 */
 if (!qemu_cpu_is_self(cpu)) {
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index b3f4b79318..288a2bd57e 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -438,7 +438,7 @@ struct CPUState {
 
 struct hax_vcpu_state *hax_vcpu;
 
-int hvf_fd;
+unsigned hvf_fd;
 
 /* track IOMMUs whose translations we've cached in the TCG TLB */
 GArray *iommu_notifiers;
diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index 1d40a8ec01..aaa00cbf05 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -25,6 +25,7 @@ extern bool hvf_allowed;
 
 int hvf_init_vcpu(CPUState *);
 int hvf_vcpu_exec(CPUState *);
+void hvf_vcpu_kick(CPUState *);
 void hvf_cpu_synchronize_state(CPUState *);
 void hvf_cpu_synchronize_post_reset(CPUState *);
 void hvf_cpu_synchronize_post_init(CPUState *);
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index efe9802962..4d254a477a 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -966,6 +966,17 @@ int hvf_vcpu_exec(CPUState *cpu)
 return ret;
 }
 
+void hvf_vcpu_kick(CPUState *cpu)
+{
+hv_return_t err;
+
+err = hv_vcpu_interrupt(&cpu->hvf_fd, 1);
+if (err) {
+fprintf(stderr, "qemu:%s error %#x\n", __func__, err);
+exit(1);
+}
+}
+
 bool hvf_allowed;
 
 static int hvf_accel_init(MachineState *ms)
-- 
2.26.1




[PATCH 8/8] MAINTAINERS: Add Cameron as HVF co-maintainer

2020-06-24 Thread Roman Bolshakov
Similar patch was sent a while ago but got lost.
While at it, add a status wiki page.

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 01e6b3fefe..f54a50cdb2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -417,7 +417,9 @@ F: target/i386/kvm.c
 F: scripts/kvm/vmxcap
 
 X86 HVF CPUs
+M: Cameron Esfahani 
 M: Roman Bolshakov 
+W: https://wiki.qemu.org/Features/HVF
 S: Maintained
 F: accel/stubs/hvf-stub.c
 F: target/i386/hvf/
-- 
2.26.1




[Bug 1818937] Re: Crash with HV_ERROR on macOS host

2020-06-24 Thread Roman Bolshakov
The issue should be fixed in QEMU v5.0+

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1818937

Title:
  Crash with HV_ERROR on macOS host

Status in QEMU:
  Fix Released

Bug description:
  On macOS host running Windows 10 guest, qemu crashed with error
  message: Error: HV_ERROR.

  Host: macOS Mojave 10.14.3 (18D109) Late 2014 Mac mini presumably Core i5 
4278U.
  QEMU: git commit a3e3b0a7bd5de211a62cdf2d6c12b96d3c403560
  QEMU parameter: qemu-system-x86_64 -m 3000 -drive 
file=disk.img,if=virtio,discard=unmap -accel hvf -soundhw hda -smp 3

  thread list
  Process 56054 stopped
thread #1: tid = 0x2ffec8, 0x7fff48d0805a vImage`vLookupTable_Planar16 
+ 970, queue = 'com.apple.main-thread'
thread #2: tid = 0x2ffecc, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #3: tid = 0x2ffecd, 0x7fff79d715aa 
libsystem_kernel.dylib`__select + 10
thread #4: tid = 0x2ffece, 0x7fff79d71d9a 
libsystem_kernel.dylib`__sigwait + 10
  * thread #6: tid = 0x2ffed0, 0x7fff79d7023e 
libsystem_kernel.dylib`__pthread_kill + 10, stop reason = signal SIGABRT
thread #7: tid = 0x2ffed1, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #8: tid = 0x2ffed2, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #11: tid = 0x2fff34, 0x7fff79d6a17a 
libsystem_kernel.dylib`mach_msg_trap + 10, name = 'com.apple.NSEventThread'
thread #30: tid = 0x300c04, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #31: tid = 0x300c16, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #32: tid = 0x300c17, 0x
thread #33: tid = 0x300c93, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10

  
  Crashed thread:

  * thread #6, stop reason = signal SIGABRT
* frame #0: 0x7fff79d7023e libsystem_kernel.dylib`__pthread_kill + 10
  frame #1: 0x7fff79e26c1c libsystem_pthread.dylib`pthread_kill + 285
  frame #2: 0x7fff79cd91c9 libsystem_c.dylib`abort + 127
  frame #3: 0x00010baa476d 
qemu-system-x86_64`assert_hvf_ok(ret=) at hvf.c:106 [opt]
  frame #4: 0x00010baa4c8f 
qemu-system-x86_64`hvf_vcpu_exec(cpu=0x7f8e5283de00) at hvf.c:681 [opt]
  frame #5: 0x00010b988423 
qemu-system-x86_64`qemu_hvf_cpu_thread_fn(arg=0x7f8e5283de00) at 
cpus.c:1636 [opt]
  frame #6: 0x00010bd9dfce 
qemu-system-x86_64`qemu_thread_start(args=) at 
qemu-thread-posix.c:502 [opt]
  frame #7: 0x7fff79e24305 libsystem_pthread.dylib`_pthread_body + 126
  frame #8: 0x7fff79e2726f libsystem_pthread.dylib`_pthread_start + 70
  frame #9: 0x7fff79e23415 libsystem_pthread.dylib`thread_start + 13

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1818937/+subscriptions



[Bug 1818937] Re: Crash with HV_ERROR on macOS host

2020-06-24 Thread Roman Bolshakov
I'm not exactly sure what commit improved the situation (either
https://git.qemu.org/?p=qemu.git;a=commit;h=e37aa8b0e410 or
https://git.qemu.org/?p=qemu.git;a=commit;h=fbafbb6db7742)  but I have
noticed that sporadic failures are gone after the series was applied:
https://lists.gnu.org/archive/html/qemu-devel/2019-11/msg03977.html

** Changed in: qemu
   Status: New => Fix Released

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1818937

Title:
  Crash with HV_ERROR on macOS host

Status in QEMU:
  Fix Released

Bug description:
  On macOS host running Windows 10 guest, qemu crashed with error
  message: Error: HV_ERROR.

  Host: macOS Mojave 10.14.3 (18D109) Late 2014 Mac mini presumably Core i5 
4278U.
  QEMU: git commit a3e3b0a7bd5de211a62cdf2d6c12b96d3c403560
  QEMU parameter: qemu-system-x86_64 -m 3000 -drive 
file=disk.img,if=virtio,discard=unmap -accel hvf -soundhw hda -smp 3

  thread list
  Process 56054 stopped
thread #1: tid = 0x2ffec8, 0x7fff48d0805a vImage`vLookupTable_Planar16 
+ 970, queue = 'com.apple.main-thread'
thread #2: tid = 0x2ffecc, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #3: tid = 0x2ffecd, 0x7fff79d715aa 
libsystem_kernel.dylib`__select + 10
thread #4: tid = 0x2ffece, 0x7fff79d71d9a 
libsystem_kernel.dylib`__sigwait + 10
  * thread #6: tid = 0x2ffed0, 0x7fff79d7023e 
libsystem_kernel.dylib`__pthread_kill + 10, stop reason = signal SIGABRT
thread #7: tid = 0x2ffed1, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #8: tid = 0x2ffed2, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10
thread #11: tid = 0x2fff34, 0x7fff79d6a17a 
libsystem_kernel.dylib`mach_msg_trap + 10, name = 'com.apple.NSEventThread'
thread #30: tid = 0x300c04, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #31: tid = 0x300c16, 0x7fff79e233f8 
libsystem_pthread.dylib`start_wqthread
thread #32: tid = 0x300c17, 0x
thread #33: tid = 0x300c93, 0x7fff79d6d7de 
libsystem_kernel.dylib`__psynch_cvwait + 10

  
  Crashed thread:

  * thread #6, stop reason = signal SIGABRT
* frame #0: 0x7fff79d7023e libsystem_kernel.dylib`__pthread_kill + 10
  frame #1: 0x7fff79e26c1c libsystem_pthread.dylib`pthread_kill + 285
  frame #2: 0x7fff79cd91c9 libsystem_c.dylib`abort + 127
  frame #3: 0x00010baa476d 
qemu-system-x86_64`assert_hvf_ok(ret=) at hvf.c:106 [opt]
  frame #4: 0x00010baa4c8f 
qemu-system-x86_64`hvf_vcpu_exec(cpu=0x7f8e5283de00) at hvf.c:681 [opt]
  frame #5: 0x00010b988423 
qemu-system-x86_64`qemu_hvf_cpu_thread_fn(arg=0x7f8e5283de00) at 
cpus.c:1636 [opt]
  frame #6: 0x00010bd9dfce 
qemu-system-x86_64`qemu_thread_start(args=) at 
qemu-thread-posix.c:502 [opt]
  frame #7: 0x7fff79e24305 libsystem_pthread.dylib`_pthread_body + 126
  frame #8: 0x7fff79e2726f libsystem_pthread.dylib`_pthread_start + 70
  frame #9: 0x7fff79e23415 libsystem_pthread.dylib`thread_start + 13

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1818937/+subscriptions



Re: [PATCH] trivial: Remove trailing whitespaces

2020-07-08 Thread Roman Bolshakov
On Mon, Jul 06, 2020 at 06:23:00PM +0200, Christophe de Dinechin wrote:
> There are a number of unnecessary trailing whitespaces that have
> accumulated over time in the source code. They cause stray changes
> in patches if you use tools that automatically remove them.
> 
> Tested by doing a `git diff -w` after the change.
> 
> This could probably be turned into a pre-commit hook.
> 

For HVF bits,

Reviewed-by: Roman Bolshakov 

Thanks,
Roman



[Bug 1840719] Re: win98se floppy fails to boot with isapc machine

2020-07-12 Thread Roman Bolshakov
The commit fixes the issue in master branch:
https://git.qemu.org/?p=qemu.git;a=commit;h=de15df5ead400b7c3d0cf21c8164a7686dc81933

The fix is going to be released in 5.1

** Changed in: qemu
   Status: In Progress => Fix Committed

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1840719

Title:
  win98se floppy fails to boot with isapc machine

Status in QEMU:
  Fix Committed

Bug description:
  QEMU emulator version 4.1.50 (commit 50d69ee0d)

  floppy image from:
  https://winworldpc.com/download/417d71c2-ae18-c39a-11c3-a4e284a2c3a5

  $ qemu-system-i386 -M isapc -fda Windows\ 98\ Second\ Edition\ Boot.img
  SeaBIOS (version rel-1.12.1-0...)
  Booting from Floppy...
  Boot failed: could not read the boot disk

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1840719/+subscriptions



Re: [PATCH v3] i386: hvf: Implement CPU kick

2020-07-13 Thread Roman Bolshakov
On Thu, Jul 02, 2020 at 02:42:45PM +0200, Paolo Bonzini wrote:
> On 02/07/20 12:57, Roman Bolshakov wrote:
> > There's still a small chance of kick loss, on user-to-kernel border
> > between atomic_mb_set's just before the entry to hv_vcpu_run and just
> > after it.
> 
> Good point, but we can fix it.
> 
> > -static void dummy_signal(int sig)
> > +static void hvf_handle_ipi(int sig)
> >  {
> > +CPUState *cpu = pthread_getspecific(hvf_cpu);
> 
> You can use current_cpu here.  If it's NULL, just return (it's a
> per-thread variable).
> 
> > +X86CPU *x86_cpu = X86_CPU(cpu);
> > +CPUX86State *env = &x86_cpu->env;
> > +
> > +if (!atomic_xchg(&env->hvf_in_guest, false)) {
> 
> Here, thinking more about it, we need not write hvf_in_guest, so:
> 
>   /* Write cpu->exit_request before reading env->hvf_in_guest.  */
>   smp_mb();
>   if (!atomic_read(&env->hvf_in_guest)) {
>   ...
>   }
> 
> > +wvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS,
> > +  rvmcs(cpu->hvf_fd, VMCS_PIN_BASED_CTLS)
> > +| VMCS_PIN_BASED_CTLS_VMX_PREEMPT_TIMER);
> > +}
> >  }
> >  
> >  int hvf_init_vcpu(CPUState *cpu)
> > @@ -631,7 +650,9 @@ int hvf_vcpu_exec(CPUState *cpu)
> >  return EXCP_HLT;
> >  }
> >  
> > +atomic_mb_set(&env->hvf_in_guest, true);
> >  hv_return_t r  = hv_vcpu_run(cpu->hvf_fd);
> > +atomic_mb_set(&env->hvf_in_guest, false);
> 
> 
> And here you can do instead:
> 
>   atomic_set(&env->hvf_in_guest, true);
>   /* Read cpu->exit_request after writing env->hvf_in_guest.  */
>   smp_mb();
>   if (atomic_read(&cpu->exit_request)) {
>   qemu_mutex_lock_iothread();
>   atomic_set(&env->hvf_in_guest, false);
>   return EXCP_INTERRUPT;
>   }
>   hv_return_t r  = hv_vcpu_run(cpu->hvf_fd);
>   atomic_store_release(&env->hvf_in_guest, false);
> 
> This matching "write A/smp_mb()/read B" and "write B/smp_mb()/read A" is
> a very common idiom for lock-free signaling between threads.
> 

Hi Paolo,

Thanks for the feedback and the guidelines. I think I've got the idea:

exit_request is the way to record the fact of kick request even if it
was sent outside of hv_vcpu_run().

Best regards,
Roman



[PATCH for-5.1] i386: hvf: Explicitly set CR4 guest/host mask

2020-07-14 Thread Roman Bolshakov
Removal of register reset omitted initialization of CR4 guest/host mask.
x86_64 guests aren't booting without it.

Fixes: 5009ef22c6bb2 ("i386: hvf: Don't duplicate register reset")
Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/vmx.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index 75ba1e2a5f..587b1b8375 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -166,6 +166,7 @@ static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t 
cr4)
 
 wvmcs(vcpu, VMCS_GUEST_CR4, guest_cr4);
 wvmcs(vcpu, VMCS_CR4_SHADOW, cr4);
+wvmcs(vcpu, VMCS_CR4_MASK, CR4_VMXE);
 
 hv_vcpu_invalidate_tlb(vcpu);
 hv_vcpu_flush(vcpu);
-- 
2.26.1




Re: [PATCH for-5.1] i386: hvf: Explicitly set CR4 guest/host mask

2020-07-14 Thread Roman Bolshakov
On Tue, Jul 14, 2020 at 12:07:27PM +0300, Roman Bolshakov wrote:
> Removal of register reset omitted initialization of CR4 guest/host mask.
> x86_64 guests aren't booting without it.
> 
> Fixes: 5009ef22c6bb2 ("i386: hvf: Don't duplicate register reset")
> Signed-off-by: Roman Bolshakov 
> 

If one has a chance to test or review it, it'd be very helpful. That'd
allow to include it in RC0.

Thanks,
Roman



Re: [PATCH-for-5.1] net/colo: Remove unused trace event

2020-07-15 Thread Roman Bolshakov
On Wed, Jul 15, 2020 at 04:31:30PM +0200, Philippe Mathieu-Daudé wrote:
> Unused trace event cause build failure when using the dtrace backend:
> 
>   "probe colo_compare_miscompare doesn't exist"
> 
> Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison")
> Reported-by: Roman Bolshakov 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Cc: zhangchen.f...@cn.fujitsu.com
> ---
>  net/trace-events | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/net/trace-events b/net/trace-events
> index fa49c71533..c3f623d40c 100644
> --- a/net/trace-events
> +++ b/net/trace-events
> @@ -17,7 +17,6 @@ colo_compare_udp_miscompare(const char *sta, int size) ": 
> %s = %d"
>  colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
>  colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, 
> const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, 
> spkt size = %d, ip_src = %s, ip_dst = %s"
>  colo_old_packet_check_found(int64_t old_time) "%" PRId64
> -colo_compare_miscompare(void) ""
>  colo_compare_tcp_info(const char *pkt, uint32_t seq, uint32_t ack, int 
> hdlen, int pdlen, int offset, int flags) "%s: seq/ack= %u/%u hdlen= %d pdlen= 
> %d offset= %d flags=%d"
>  
>  # filter-rewriter.c
> -- 
> 2.21.3
> 

Hi Philippe,

Thanks for submitting it but this is not enough, here's the patch that
works (but this is only one of the patches that enables dtrace on macOS):

diff --git a/net/trace-events b/net/trace-events
index fa49c71533..c3f623d40c 100644
--- a/net/trace-events
+++ b/net/trace-events
@@ -17,7 +17,6 @@ colo_compare_udp_miscompare(const char *sta, int size) ": %s 
= %d"
 colo_compare_icmp_miscompare(const char *sta, int size) ": %s = %d"
 colo_compare_ip_info(int psize, const char *sta, const char *stb, int ssize, 
const char *stc, const char *std) "ppkt size = %d, ip_src = %s, ip_dst = %s, 
spkt size = %d, ip_src = %s, ip_dst = %s"
 colo_old_packet_check_found(int64_t old_time) "%" PRId64
-colo_compare_miscompare(void) ""
 colo_compare_tcp_info(const char *pkt, uint32_t seq, uint32_t ack, int hdlen, 
int pdlen, int offset, int flags) "%s: seq/ack= %u/%u hdlen= %d pdlen= %d 
offset= %d flags=%d"

 # filter-rewriter.c
diff --git a/net/colo-compare.c b/net/colo-compare.c
index 398b7546ff..9525ed703b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -327,7 +327,7 @@ static int colo_compare_packet_payload(Packet *ppkt,
uint16_t len)

 {
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) {
 char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];

 strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
@@ -492,7 +492,7 @@ sec:
 g_queue_push_head(&conn->primary_list, ppkt);
 g_queue_push_head(&conn->secondary_list, spkt);

-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_TCP_INFO)) {
 qemu_hexdump((char *)ppkt->data, stderr,
 "colo-compare ppkt", ppkt->size);
 qemu_hexdump((char *)spkt->data, stderr,
@@ -533,7 +533,8 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
*ppkt)
 ppkt->size - offset)) {
 trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
 trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(
+  TRACE_COLO_COMPARE_UDP_MISCOMPARE)) {
 qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
  ppkt->size);
 qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
@@ -576,7 +577,8 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet 
*ppkt)
ppkt->size);
 trace_colo_compare_icmp_miscompare("Secondary pkt size",
spkt->size);
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(
+  TRACE_COLO_COMPARE_ICMP_MISCOMPARE)) {
 qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
  ppkt->size);
 qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
@@ -597,7 +599,7 @@ static int colo_packet_compare_other(Packet 

Re: [PATCH-for-5.1] net/colo: Remove unused trace event

2020-07-15 Thread Roman Bolshakov
On Wed, Jul 15, 2020 at 04:13:02PM +0100, Daniel P. Berrangé wrote:
> On Wed, Jul 15, 2020 at 04:31:30PM +0200, Philippe Mathieu-Daudé wrote:
> > Unused trace event cause build failure when using the dtrace backend:
> > 
> >   "probe colo_compare_miscompare doesn't exist"
> > 
> > Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison")
> > Reported-by: Roman Bolshakov 
> 
> Note Roman indicated on IRC that this is seen with dtrace on macOS and
> there were other problems too.
> 
> Unless someone knows that QEMU's dtrace support worked on macOS in the
> past, I think we don't need  and simply consider dtrace unsupported on
> macOS right now until someone can get it into more robust shape on
> macOS.
> 

Hi Daniel,

I've got it working and I can see qemu trace points from
  sudo dtrace -m qemu-system-x86_64 -l

I'm wrapping a series that resolves all the issues along the way.

Thanks,
Roman



[PATCH 2/4] scripts/tracetool: Use void pointer for vcpu

2020-07-16 Thread Roman Bolshakov
dtrace on macOS complains that CPUState * is used for a few probes:

  dtrace: failed to compile script trace-dtrace-root.dtrace: line 130: syntax 
error near "CPUState"

A comment in scripts/tracetool/__init__.py mentions that:

  We only want to allow standard C types or fixed sized
  integer types. We don't want QEMU specific types
  as we can't assume trace backends can resolve all the
  typedefs

Fixes: 3d211d9f4dbee ("trace: Add 'vcpu' event property to trace guest vCPU")
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 scripts/tracetool/vcpu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/tracetool/vcpu.py b/scripts/tracetool/vcpu.py
index b54e4f4e7a..868b4cb04c 100644
--- a/scripts/tracetool/vcpu.py
+++ b/scripts/tracetool/vcpu.py
@@ -24,7 +24,7 @@ def transform_event(event):
 assert "tcg-trans" not in event.properties
 assert "tcg-exec" not in event.properties
 
-event.args = Arguments([("CPUState *", "__cpu"), event.args])
+event.args = Arguments([("void *", "__cpu"), event.args])
 if "tcg" in event.properties:
 fmt = "\"cpu=%p \""
 event.fmt = [fmt + event.fmt[0],
-- 
2.26.1




[PATCH 1/4] scripts/tracetool: Fix dtrace generation for macOS

2020-07-16 Thread Roman Bolshakov
dtrace USDT is fully supported since OS X 10.6. There are a few
peculiarities compared to other dtrace flavors.

1. It doesn't accept empty files.
2. It doesn't recognize bool type but accepts ANSI C _Bool.

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 scripts/tracetool/format/d.py | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
index 0afb5f3f47..be4a2aa254 100644
--- a/scripts/tracetool/format/d.py
+++ b/scripts/tracetool/format/d.py
@@ -13,6 +13,7 @@ __email__  = "stefa...@redhat.com"
 
 
 from tracetool import out
+from sys import platform
 
 
 # Reserved keywords from
@@ -34,7 +35,8 @@ def generate(events, backend, group):
 
 # SystemTap's dtrace(1) warns about empty "provider qemu {}" but is happy
 # with an empty file.  Avoid the warning.
-if not events:
+# But dtrace on macOS can't deal with empty files.
+if not events and platform != "darwin":
 return
 
 out('/* This file is autogenerated by tracetool, do not edit. */'
@@ -44,6 +46,11 @@ def generate(events, backend, group):
 for e in events:
 args = []
 for type_, name in e.args:
+if platform == "darwin":
+if type_ == 'bool':
+type_ = '_Bool'
+if type_ == 'bool *':
+type_ = '_Bool *'
 if name in RESERVED_WORDS:
 name += '_'
 args.append(type_ + ' ' + name)
-- 
2.26.1




[PATCH 0/4] Add dtrace support on macOS

2020-07-16 Thread Roman Bolshakov
Hi,

This is a small series that enables dtrace tracing backend on macOS.
Whether or not it should go to 5.1 is up to discretion of tracing
maintainers.

Thanks,
Roman

Roman Bolshakov (4):
  scripts/tracetool: Fix dtrace generation for macOS
  scripts/tracetool: Use void pointer for vcpu
  build: Don't make object files for dtrace on macOS
  net/colo: Match is-enabled probe to tracepoint

 Makefile.objs |  2 ++
 net/colo-compare.c| 12 +++-
 net/filter-rewriter.c |  8 ++--
 net/trace-events  |  2 --
 scripts/tracetool/format/d.py |  9 -
 scripts/tracetool/vcpu.py |  2 +-
 6 files changed, 24 insertions(+), 11 deletions(-)

-- 
2.26.1




[PATCH 4/4] net/colo: Match is-enabled probe to tracepoint

2020-07-16 Thread Roman Bolshakov
Build of QEMU with dtrace fails on macOS:

  LINKx86_64-softmmu/qemu-system-x86_64
error: probe colo_compare_miscompare doesn't exist
error: Could not register probes
ld: error creating dtrace DOF section for architecture x86_64

The reason of the error is explained by Adam Leventhal [1]:

  Note that is-enabled probes don't have the stability magic so I'm not
  sure how things would work if only is-enabled probes were used.

net/colo code uses is-enabled probes to determine if other probes should
be used but colo_compare_miscompare itself is not used explicitly.
Linker doesn't include the symbol and build fails.

The issue can be resolved if is-enabled probe matches the actual trace
point that is used inside the test.

1. http://markmail.org/message/6grq2ygr5nwdwsnb

Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison")
Cc: Philippe Mathieu-Daudé 
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 net/colo-compare.c| 12 +++-
 net/filter-rewriter.c |  8 ++--
 net/trace-events  |  2 --
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 398b7546ff..9525ed703b 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -327,7 +327,7 @@ static int colo_compare_packet_payload(Packet *ppkt,
uint16_t len)
 
 {
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) {
 char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
 
 strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
@@ -492,7 +492,7 @@ sec:
 g_queue_push_head(&conn->primary_list, ppkt);
 g_queue_push_head(&conn->secondary_list, spkt);
 
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_TCP_INFO)) {
 qemu_hexdump((char *)ppkt->data, stderr,
 "colo-compare ppkt", ppkt->size);
 qemu_hexdump((char *)spkt->data, stderr,
@@ -533,7 +533,8 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
*ppkt)
 ppkt->size - offset)) {
 trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
 trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(
+  TRACE_COLO_COMPARE_UDP_MISCOMPARE)) {
 qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
  ppkt->size);
 qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
@@ -576,7 +577,8 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet 
*ppkt)
ppkt->size);
 trace_colo_compare_icmp_miscompare("Secondary pkt size",
spkt->size);
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(
+  TRACE_COLO_COMPARE_ICMP_MISCOMPARE)) {
 qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
  ppkt->size);
 qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
@@ -597,7 +599,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet 
*ppkt)
 uint16_t offset = ppkt->vnet_hdr_len;
 
 trace_colo_compare_main("compare other");
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) {
 char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
 
 strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c
index 1aaad101b6..ff04165cc4 100644
--- a/net/filter-rewriter.c
+++ b/net/filter-rewriter.c
@@ -76,7 +76,9 @@ static int handle_primary_tcp_pkt(RewriterState *rf,
 struct tcp_hdr *tcp_pkt;
 
 tcp_pkt = (struct tcp_hdr *)pkt->transport_header;
-if (trace_event_get_state_backends(TRACE_COLO_FILTER_REWRITER_DEBUG)) {
+if (trace_event_get_state_backends(TRACE_COLO_FILTER_REWRITER_PKT_INFO) ||
+trace_event_get_state_backends(
+  TRACE_COLO_FILTER_REWRITER_CONN_OFFSET)) {
 trace_colo_filter_rewriter_pkt_info(__func__,
 inet_ntoa(pkt->ip->ip_src), inet_ntoa(pkt->ip->ip_dst),
 ntohl(tcp_pkt->th_seq), ntohl(tcp_pkt->th_ack),
@@ -180,7 +182,9 @@ static int handle_secondary_tcp_pkt(RewriterState *rf,
 
 tcp_pkt = (struct tcp_hdr *)pkt->t

[PATCH 3/4] build: Don't make object files for dtrace on macOS

2020-07-16 Thread Roman Bolshakov
dtrace on macOS uses unresolved symbols with a special prefix to define
probes [1], only headers should be generated for USDT (dtrace(1)). But
it doesn't support backwards compatible no-op -G flag [2] and implicit
build rules fail.

1. https://markmail.org/message/6grq2ygr5nwdwsnb
2. https://markmail.org/message/5xrxt2w5m42nojkz

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 Makefile.objs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile.objs b/Makefile.objs
index d22b3b45d7..982f15ba30 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -211,5 +211,7 @@ trace-events-files = $(SRC_PATH)/trace-events 
$(trace-events-subdirs:%=$(SRC_PAT
 trace-obj-y = trace-root.o
 trace-obj-y += $(trace-events-subdirs:%=%/trace.o)
 trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o
+ifneq ($(CONFIG_DARWIN),y)
 trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
 trace-obj-$(CONFIG_TRACE_DTRACE) += $(trace-events-subdirs:%=%/trace-dtrace.o)
+endif
-- 
2.26.1




Re: [PATCH 4/4] net/colo: Match is-enabled probe to tracepoint

2020-07-16 Thread Roman Bolshakov
On Thu, Jul 16, 2020 at 09:51:27AM +0100, Daniel P. Berrangé wrote:
> Adding Stefan to CC as the trace maintainer.
> 
> On Thu, Jul 16, 2020 at 11:17:54AM +0300, Roman Bolshakov wrote:
> > Build of QEMU with dtrace fails on macOS:
> > 
> >   LINKx86_64-softmmu/qemu-system-x86_64
> > error: probe colo_compare_miscompare doesn't exist
> > error: Could not register probes
> > ld: error creating dtrace DOF section for architecture x86_64
> > 
> > The reason of the error is explained by Adam Leventhal [1]:
> > 
> >   Note that is-enabled probes don't have the stability magic so I'm not
> >   sure how things would work if only is-enabled probes were used.
> > 
> > net/colo code uses is-enabled probes to determine if other probes should
> > be used but colo_compare_miscompare itself is not used explicitly.
> > Linker doesn't include the symbol and build fails.
> > 
> > The issue can be resolved if is-enabled probe matches the actual trace
> > point that is used inside the test.
> > 
> > 1. http://markmail.org/message/6grq2ygr5nwdwsnb
> > 
> > Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison")
> > Cc: Philippe Mathieu-Daudé 
> > Cc: Cameron Esfahani 
> > Signed-off-by: Roman Bolshakov 
> > ---
> >  net/colo-compare.c| 12 +++-
> >  net/filter-rewriter.c |  8 ++--
> >  net/trace-events  |  2 --
> >  3 files changed, 13 insertions(+), 9 deletions(-)
> > 
> > diff --git a/net/colo-compare.c b/net/colo-compare.c
> > index 398b7546ff..9525ed703b 100644
> > --- a/net/colo-compare.c
> > +++ b/net/colo-compare.c
> > @@ -327,7 +327,7 @@ static int colo_compare_packet_payload(Packet *ppkt,
> > uint16_t len)
> >  
> >  {
> > -if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
> > +if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) {
> >  char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], 
> > sec_ip_dst[20];
> >  
> >  strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
> > @@ -492,7 +492,7 @@ sec:
> >  g_queue_push_head(&conn->primary_list, ppkt);
> >  g_queue_push_head(&conn->secondary_list, spkt);
> >  
> > -if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) 
> > {
> > +if (trace_event_get_state_backends(TRACE_COLO_COMPARE_TCP_INFO)) {
> >  qemu_hexdump((char *)ppkt->data, stderr,
> >  "colo-compare ppkt", ppkt->size);
> >  qemu_hexdump((char *)spkt->data, stderr,
> 
> Not your fault, as this problem is pre-existing, but IMHO this block of code
> is simply broken by design. It is checking a trace event enablement state,
> and then not emitting any trace event, but instead dumping info to stderr.
> This is mis-use of the trace framework, and changing the event name fixes
> your immediate macOS bug but the code is still flawed.
> 
> Basically it is using the trace framework as a way to dynamically turn on /
> off general debugging information.
> 
> I'm not quite sure what todo to fix it, but I don't think it should be wrapped
> in any trace_event_get_state_backends() call at all.
> 
> The simplest immediate option I think is to change it to be a compile time
> debug
> 
>   // #define DEBUG_COLO_PACKETS
> 
> and then use
> 
>   #ifdef DEBUG_COLO_PACKETS
>   qemu_hexdump(...)
>   #endif
> 
> and then leave it upto the maintainer to come up with a more advanced
> solution if they want to make it runtime configurable again, but not
> abusing the trace framework.
> 


Hi Daniel,

It makes sense, compile-time define works better because the trace data
doesn't go into a trace point. I'll use that approach in v2.

> > @@ -533,7 +533,8 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
> > *ppkt)
> >  ppkt->size - offset)) {
> >  trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> >  trace_colo_compare_udp_miscompare("Secondary pkt size", 
> > spkt->size);
> > -if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) 
> > {
> > +if (trace_event_get_state_backends(
> > +  TRACE_COLO_COMPARE_UDP_MISCOMPARE)) {
> >  qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri 
> > pkt",
> >   ppkt->size);
> > 

Re: [PATCH for-5.1] i386: hvf: Explicitly set CR4 guest/host mask

2020-07-16 Thread Roman Bolshakov
On Thu, Jul 16, 2020 at 02:14:57PM -0400, Eduardo Habkost wrote:
> On Tue, Jul 14, 2020 at 08:20:04PM +0200, Paolo Bonzini wrote:
> > Hi Roman, please ask Peter to apply it directly because I won't be able to
> > send a pull request in the next couple of weeks.
> > 
> > Paolo
> > 
> > Il mar 14 lug 2020, 12:39 Roman Bolshakov  ha
> > scritto:
> > 
> > > On Tue, Jul 14, 2020 at 12:07:27PM +0300, Roman Bolshakov wrote:
> > > > Removal of register reset omitted initialization of CR4 guest/host mask.
> > > > x86_64 guests aren't booting without it.
> > > >
> > > > Fixes: 5009ef22c6bb2 ("i386: hvf: Don't duplicate register reset")
> > > > Signed-off-by: Roman Bolshakov 
> > > >
> > >
> > > If one has a chance to test or review it, it'd be very helpful. That'd
> > > allow to include it in RC0.
> > >
> 
> I'll queue it for my -rc1 pull request.
> 

Thanks!

-Roman



[PATCH v2 1/4] scripts/tracetool: Fix dtrace generation for macOS

2020-07-17 Thread Roman Bolshakov
dtrace USDT is fully supported since OS X 10.6. There are a few
peculiarities compared to other dtrace flavors.

1. It doesn't accept empty files.
2. It doesn't recognize bool type but accepts C99 _Bool.
3. It converts int8_t * in probe points to char * in
   header files and introduces [-Wpointer-sign] warning.

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 scripts/tracetool/format/d.py | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
index 0afb5f3f47..353722f89c 100644
--- a/scripts/tracetool/format/d.py
+++ b/scripts/tracetool/format/d.py
@@ -13,6 +13,7 @@ __email__  = "stefa...@redhat.com"
 
 
 from tracetool import out
+from sys import platform
 
 
 # Reserved keywords from
@@ -34,7 +35,8 @@ def generate(events, backend, group):
 
 # SystemTap's dtrace(1) warns about empty "provider qemu {}" but is happy
 # with an empty file.  Avoid the warning.
-if not events:
+# But dtrace on macOS can't deal with empty files.
+if not events and platform != "darwin":
 return
 
 out('/* This file is autogenerated by tracetool, do not edit. */'
@@ -44,6 +46,17 @@ def generate(events, backend, group):
 for e in events:
 args = []
 for type_, name in e.args:
+if platform == "darwin":
+# macOS dtrace accepts only C99 _Bool
+if type_ == 'bool':
+type_ = '_Bool'
+if type_ == 'bool *':
+type_ = '_Bool *'
+# It converts int8_t * in probe points to char * in header
+# files and introduces [-Wpointer-sign] warning.
+# Avoid it by changing probe type to signed char * beforehand.
+if type_ == 'int8_t *':
+type_ = 'signed char *'
 if name in RESERVED_WORDS:
 name += '_'
 args.append(type_ + ' ' + name)
-- 
2.26.1




[PATCH v2 0/4] Add dtrace support on macOS

2020-07-17 Thread Roman Bolshakov
Hi,

This is a small series that enables dtrace tracing backend on macOS.
Whether or not it should go to 5.1 is up to discretion of tracing
maintainers.

Thanks,
Roman

Changes since v1:
 - Fixed a typo ANSI C to C99, wrt to _Bool in the first patch.
 - Prevented a few [-Wpointer-sign] warnings by converting int8_t * to
   signed char * in static probe definitions.
 - Moved COLO packet dump under #ifdef DEBUG_COLO_PACKETS (Daniel).
 - Separated tracepoints in net/filter-rewriter.c to use matching
   is-enabled probe (Daniel).

Roman Bolshakov (4):
  scripts/tracetool: Fix dtrace generation for macOS
  scripts/tracetool: Use void pointer for vcpu
  build: Don't make object files for dtrace on macOS
  net/colo: Match is-enabled probe to tracepoint

 Makefile.objs |  2 ++
 net/colo-compare.c| 42 ++-
 net/filter-rewriter.c | 10 +++--
 net/trace-events  |  2 --
 scripts/tracetool/format/d.py | 15 -
 scripts/tracetool/vcpu.py |  2 +-
 6 files changed, 47 insertions(+), 26 deletions(-)

-- 
2.26.1




[PATCH v2 4/4] net/colo: Match is-enabled probe to tracepoint

2020-07-17 Thread Roman Bolshakov
Build of QEMU with dtrace fails on macOS:

  LINKx86_64-softmmu/qemu-system-x86_64
error: probe colo_compare_miscompare doesn't exist
error: Could not register probes
ld: error creating dtrace DOF section for architecture x86_64

The reason of the error is explained by Adam Leventhal [1]:

  Note that is-enabled probes don't have the stability magic so I'm not
  sure how things would work if only is-enabled probes were used.

net/colo code uses is-enabled probes to determine if other probes should
be used but colo_compare_miscompare itself is not used explicitly.
Linker doesn't include the symbol and build fails.

The issue can be resolved if is-enabled probe matches the actual trace
point that is used inside the test. Packet dump toggle is replaced with
a compile-time conditional definition.

1. http://markmail.org/message/6grq2ygr5nwdwsnb

Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison")
Cc: Philippe Mathieu-Daudé 
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 net/colo-compare.c| 42 ++
 net/filter-rewriter.c | 10 --
 net/trace-events  |  2 --
 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index 398b7546ff..e0be98e494 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -54,6 +54,8 @@ static NotifierList colo_compare_notifiers =
 #define REGULAR_PACKET_CHECK_MS 3000
 #define DEFAULT_TIME_OUT_MS 3000
 
+/* #define DEBUG_COLO_PACKETS */
+
 static QemuMutex colo_compare_mutex;
 static bool colo_compare_active;
 static QemuMutex event_mtx;
@@ -327,7 +329,7 @@ static int colo_compare_packet_payload(Packet *ppkt,
uint16_t len)
 
 {
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
+if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO)) {
 char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], sec_ip_dst[20];
 
 strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src));
@@ -492,12 +494,12 @@ sec:
 g_queue_push_head(&conn->primary_list, ppkt);
 g_queue_push_head(&conn->secondary_list, spkt);
 
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
-qemu_hexdump((char *)ppkt->data, stderr,
-"colo-compare ppkt", ppkt->size);
-qemu_hexdump((char *)spkt->data, stderr,
-"colo-compare spkt", spkt->size);
-}
+#ifdef DEBUG_COLO_PACKETS
+qemu_hexdump((char *)ppkt->data, stderr,
+ "colo-compare ppkt", ppkt->size);
+qemu_hexdump((char *)spkt->data, stderr,
+ "colo-compare spkt", spkt->size);
+#endif
 
 colo_compare_inconsistency_notify(s);
 }
@@ -533,12 +535,12 @@ static int colo_packet_compare_udp(Packet *spkt, Packet 
*ppkt)
 ppkt->size - offset)) {
 trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
 trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
-qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
- ppkt->size);
-qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
- spkt->size);
-}
+#ifdef DEBUG_COLO_PACKETS
+qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
+ ppkt->size);
+qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
+ spkt->size);
+#endif
 return -1;
 } else {
 return 0;
@@ -576,12 +578,12 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet 
*ppkt)
ppkt->size);
 trace_colo_compare_icmp_miscompare("Secondary pkt size",
spkt->size);
-if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
-qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
- ppkt->size);
-qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
- spkt->size);
-}
+#ifdef DEBUG_COLO_PACKETS
+qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
+ ppkt->size);
+qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
+ spkt->size);
+#endif
 return -1;
 } else {
 return 0;
@@ -597,7 +599,7 @@ static int colo_packet_compare_other(Packet *spkt, Packet 

[PATCH v2 2/4] scripts/tracetool: Use void pointer for vcpu

2020-07-17 Thread Roman Bolshakov
dtrace on macOS complains that CPUState * is used for a few probes:

  dtrace: failed to compile script trace-dtrace-root.dtrace: line 130: syntax 
error near "CPUState"

A comment in scripts/tracetool/__init__.py mentions that:

  We only want to allow standard C types or fixed sized
  integer types. We don't want QEMU specific types
  as we can't assume trace backends can resolve all the
  typedefs

Fixes: 3d211d9f4dbee ("trace: Add 'vcpu' event property to trace guest vCPU")
Reviewed-by: Daniel P. Berrangé 
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 scripts/tracetool/vcpu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/tracetool/vcpu.py b/scripts/tracetool/vcpu.py
index b54e4f4e7a..868b4cb04c 100644
--- a/scripts/tracetool/vcpu.py
+++ b/scripts/tracetool/vcpu.py
@@ -24,7 +24,7 @@ def transform_event(event):
 assert "tcg-trans" not in event.properties
 assert "tcg-exec" not in event.properties
 
-event.args = Arguments([("CPUState *", "__cpu"), event.args])
+event.args = Arguments([("void *", "__cpu"), event.args])
 if "tcg" in event.properties:
 fmt = "\"cpu=%p \""
 event.fmt = [fmt + event.fmt[0],
-- 
2.26.1




[PATCH v2 3/4] build: Don't make object files for dtrace on macOS

2020-07-17 Thread Roman Bolshakov
dtrace on macOS uses unresolved symbols with a special prefix to define
probes [1], only headers should be generated for USDT (dtrace(1)). But
it doesn't support backwards compatible no-op -G flag [2] and implicit
build rules fail.

1. https://markmail.org/message/6grq2ygr5nwdwsnb
2. https://markmail.org/message/5xrxt2w5m42nojkz

Reviewed-by: Daniel P. Berrangé 
Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 Makefile.objs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile.objs b/Makefile.objs
index d22b3b45d7..982f15ba30 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -211,5 +211,7 @@ trace-events-files = $(SRC_PATH)/trace-events 
$(trace-events-subdirs:%=$(SRC_PAT
 trace-obj-y = trace-root.o
 trace-obj-y += $(trace-events-subdirs:%=%/trace.o)
 trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o
+ifneq ($(CONFIG_DARWIN),y)
 trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
 trace-obj-$(CONFIG_TRACE_DTRACE) += $(trace-events-subdirs:%=%/trace-dtrace.o)
+endif
-- 
2.26.1




Re: [PATCH v2 1/4] scripts/tracetool: Fix dtrace generation for macOS

2020-07-20 Thread Roman Bolshakov
On Sun, Jul 19, 2020 at 03:52:08PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/17/20 11:35 AM, Roman Bolshakov wrote:
> > dtrace USDT is fully supported since OS X 10.6. There are a few
> > peculiarities compared to other dtrace flavors.
> > 
> > 1. It doesn't accept empty files.
> > 2. It doesn't recognize bool type but accepts C99 _Bool.
> > 3. It converts int8_t * in probe points to char * in
> >header files and introduces [-Wpointer-sign] warning.
> > 
> > Cc: Cameron Esfahani 
> > Signed-off-by: Roman Bolshakov 
> > ---
> >  scripts/tracetool/format/d.py | 15 ++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
> > index 0afb5f3f47..353722f89c 100644
> > --- a/scripts/tracetool/format/d.py
> > +++ b/scripts/tracetool/format/d.py
> > @@ -13,6 +13,7 @@ __email__  = "stefa...@redhat.com"
> >  
> >  
> >  from tracetool import out
> > +from sys import platform
> >  
> >  
> >  # Reserved keywords from
> > @@ -34,7 +35,8 @@ def generate(events, backend, group):
> >  
> >  # SystemTap's dtrace(1) warns about empty "provider qemu {}" but is 
> > happy
> >  # with an empty file.  Avoid the warning.
> > -if not events:
> > +# But dtrace on macOS can't deal with empty files.
> > +if not events and platform != "darwin":
> 
> or?

no, the event list is empty for some files where all events are
disabled (e.g. hppa/trace-events), so it should have an "and" here. This
limits early exit only on macOS. The precendence looks correct:

https://docs.python.org/3/reference/expressions.html#operator-precedence

> 
> >  return
> >  
> >  out('/* This file is autogenerated by tracetool, do not edit. */'
> > @@ -44,6 +46,17 @@ def generate(events, backend, group):
> >  for e in events:
> >  args = []
> >  for type_, name in e.args:
> > +if platform == "darwin":
> > +# macOS dtrace accepts only C99 _Bool
> 
> Why not do that for all platforms?
> 

Because I can only test the changes on darwin :)
I don't know how other dtrace flavors behave and whether it is an issue
for dtrace on Linux/Solaris/FreeBSD/etc.

Thanks,
Roman

> > +if type_ == 'bool':
> > +type_ = '_Bool'
> > +if type_ == 'bool *':
> > +type_ = '_Bool *'
> > +# It converts int8_t * in probe points to char * in header
> > +# files and introduces [-Wpointer-sign] warning.
> > +# Avoid it by changing probe type to signed char * 
> > beforehand.
> > +if type_ == 'int8_t *':
> > +type_ = 'signed char *'
> >  if name in RESERVED_WORDS:
> >  name += '_'
> >  args.append(type_ + ' ' + name)
> > 
> 



Re: [PATCH v2 4/4] net/colo: Match is-enabled probe to tracepoint

2020-07-20 Thread Roman Bolshakov
On Sat, Jul 18, 2020 at 05:58:56PM +, Zhang, Chen wrote:
> > -Original Message-
> > From: Roman Bolshakov 
> > Sent: Friday, July 17, 2020 5:35 PM
> > @@ -533,12 +535,12 @@ static int colo_packet_compare_udp(Packet *spkt,
> > Packet *ppkt)
> >  ppkt->size - offset)) {
> >  trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> >  trace_colo_compare_udp_miscompare("Secondary pkt size", spkt-
> > >size);
> > -if
> > (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE))
> > {
> > -qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri 
> > pkt",
> > - ppkt->size);
> > -qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec 
> > pkt",
> > - spkt->size);
> > -}
> > +#ifdef DEBUG_COLO_PACKETS
> > +qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
> > + ppkt->size);
> > +qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
> > + spkt->size);
> > +#endif
> 
> I think change the " trace_event_get_state_backends()" to 
> "trace_colo_compare_main("Dump packet hex: ")" is a better choice here.
> It will keep the original code logic and avoid the problem here.
> 

Hi Chen,

Do you mean to use another is-enabled probe?
e.g. change from
"if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE))"
to
"if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MAIN))"

Thanks,
Roman



Re: [PATCH v2 3/4] build: Don't make object files for dtrace on macOS

2020-07-24 Thread Roman Bolshakov
On Fri, Jul 17, 2020 at 12:35:16PM +0300, Roman Bolshakov wrote:
> dtrace on macOS uses unresolved symbols with a special prefix to define
> probes [1], only headers should be generated for USDT (dtrace(1)). But
> it doesn't support backwards compatible no-op -G flag [2] and implicit
> build rules fail.
> 
> 1. https://markmail.org/message/6grq2ygr5nwdwsnb
> 2. https://markmail.org/message/5xrxt2w5m42nojkz
> 
> Reviewed-by: Daniel P. Berrangé 
> Cc: Cameron Esfahani 
> Signed-off-by: Roman Bolshakov 
> ---
>  Makefile.objs | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile.objs b/Makefile.objs
> index d22b3b45d7..982f15ba30 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -211,5 +211,7 @@ trace-events-files = $(SRC_PATH)/trace-events 
> $(trace-events-subdirs:%=$(SRC_PAT
>  trace-obj-y = trace-root.o
>  trace-obj-y += $(trace-events-subdirs:%=%/trace.o)
>  trace-obj-$(CONFIG_TRACE_UST) += trace-ust-all.o
> +ifneq ($(CONFIG_DARWIN),y)
>  trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace-root.o
>  trace-obj-$(CONFIG_TRACE_DTRACE) += 
> $(trace-events-subdirs:%=%/trace-dtrace.o)
> +endif
> -- 
> 2.26.1
> 

An article about DTrace [1] mentions that FreeBSD also doesn't need that:
"On FreeBSD/Mac OS X, you do not have to generate a separate probe
object file for linking. This makes the compilation process much more
straightforward [...]"

I don't know for sure but perhaps "-G" makes dummy object files there.

1. https://www.ibm.com/developerworks/aix/library/au-dtraceprobes.html

Thanks,
Roman



Re: [PATCH v2 4/4] net/colo: Match is-enabled probe to tracepoint

2020-07-29 Thread Roman Bolshakov
On Tue, Jul 21, 2020 at 03:06:57PM +0100, Daniel P. Berrangé wrote:
> On Sat, Jul 18, 2020 at 05:58:56PM +, Zhang, Chen wrote:
> > 
> > 
> > > -Original Message-
> > > From: Roman Bolshakov 
> > > Sent: Friday, July 17, 2020 5:35 PM
> > > To: qemu-devel@nongnu.org
> > > Cc: Daniel P. Berrangé ; Stefan Hajnoczi
> > > ; Cameron Esfahani ; Roman
> > > Bolshakov ; Philippe Mathieu-Daudé
> > > ; Zhang, Chen ; Li Zhijian
> > > ; Jason Wang 
> > > Subject: [PATCH v2 4/4] net/colo: Match is-enabled probe to tracepoint
> > > 
> > > Build of QEMU with dtrace fails on macOS:
> > > 
> > >   LINKx86_64-softmmu/qemu-system-x86_64
> > > error: probe colo_compare_miscompare doesn't exist
> > > error: Could not register probes
> > > ld: error creating dtrace DOF section for architecture x86_64
> > > 
> > > The reason of the error is explained by Adam Leventhal [1]:
> > > 
> > >   Note that is-enabled probes don't have the stability magic so I'm not
> > >   sure how things would work if only is-enabled probes were used.
> > > 
> > > net/colo code uses is-enabled probes to determine if other probes should 
> > > be
> > > used but colo_compare_miscompare itself is not used explicitly.
> > > Linker doesn't include the symbol and build fails.
> > > 
> > > The issue can be resolved if is-enabled probe matches the actual trace 
> > > point
> > > that is used inside the test. Packet dump toggle is replaced with a 
> > > compile-
> > > time conditional definition.
> > > 
> > > 1. http://markmail.org/message/6grq2ygr5nwdwsnb
> > > 
> > > Fixes: f4b618360e ("colo-compare: add TCP, UDP, ICMP packet comparison")
> > > Cc: Philippe Mathieu-Daudé 
> > > Cc: Cameron Esfahani 
> > > Signed-off-by: Roman Bolshakov 
> > > ---
> > >  net/colo-compare.c| 42 ++
> > >  net/filter-rewriter.c | 10 --
> > >  net/trace-events  |  2 --
> > >  3 files changed, 30 insertions(+), 24 deletions(-)
> 
> 
> > > (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE))
> > > {
> > > +if (trace_event_get_state_backends(TRACE_COLO_COMPARE_IP_INFO))
> > > {
> > >  char pri_ip_src[20], pri_ip_dst[20], sec_ip_src[20], 
> > > sec_ip_dst[20];
> > > 
> > >  strcpy(pri_ip_src, inet_ntoa(ppkt->ip->ip_src)); @@ -492,12 
> > > +494,12
> > > @@ sec:
> > >  g_queue_push_head(&conn->primary_list, ppkt);
> > >  g_queue_push_head(&conn->secondary_list, spkt);
> > > 
> > > -if
> > > (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE))
> > > {
> > > -qemu_hexdump((char *)ppkt->data, stderr,
> > > -"colo-compare ppkt", ppkt->size);
> > > -qemu_hexdump((char *)spkt->data, stderr,
> > > -"colo-compare spkt", spkt->size);
> > > -}
> > > +#ifdef DEBUG_COLO_PACKETS
> > > +qemu_hexdump((char *)ppkt->data, stderr,
> > > + "colo-compare ppkt", ppkt->size);
> > > +qemu_hexdump((char *)spkt->data, stderr,
> > > + "colo-compare spkt", spkt->size); #endif
> > > 
> > >  colo_compare_inconsistency_notify(s);
> > >  }
> > > @@ -533,12 +535,12 @@ static int colo_packet_compare_udp(Packet *spkt,
> > > Packet *ppkt)
> > >  ppkt->size - offset)) {
> > >  trace_colo_compare_udp_miscompare("primary pkt size", 
> > > ppkt->size);
> > >  trace_colo_compare_udp_miscompare("Secondary pkt size", spkt-
> > > >size);
> > > -if
> > > (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE))
> > > {
> > > -qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri 
> > > pkt",
> > > - ppkt->size);
> > > -qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec 
> > > pkt",
> > > - spkt->size);
> > > -}
> > > +#ifdef DEBUG_COLO_PACKETS
> > > +qemu_hexdump((char 

[PATCH v4] i386: hvf: Implement CPU kick

2020-07-29 Thread Roman Bolshakov
HVF doesn't have a CPU kick and without it it's not possible to perform
an action on CPU thread until a VMEXIT happens. The kick is also needed
for timely interrupt delivery.

Existing implementation of CPU kick sends SIG_IPI (aka SIGUSR1) to vCPU
thread, but it's different from what hv_vcpu_interrupt does. The latter
one results in invocation of mp_cpus_kick() in XNU kernel [1].

mp_cpus_kick() sends an IPI through the host LAPIC to the HVF vCPU.
And the kick interrupt leads to VM exit because "external-interrupt
exiting” VM-execution control is enabled for HVF.

hv_vcpu_interrupt() has no effect if it's delivered when vCPU is outside
of a guest, therefore to avoid kick loss it's complemented with a
SIG_IPI handler and zero VMX-preemption timer. If the kick happens
outside of hv_vcpu_run(), the signal handler will re-queue the kick by
setting exit_request. exit_request is cleared when the request is
satisifed, i.e. when vCPU thread returns with EXCP_INTERRUPT.

So we get the following scenarios time/location-wise for the kick:

1) vCPU thread is far away before hv_vcpu_run(), then exit_request is
   scheduled. As soon as vCPU thread approaches hv_vcpu_run(), the
   exit request is satasified.

2) vCPU thread is about to enter the guest, then VMX-preemption timer is
   enabled to expedite immediate VM-exit. The VMX-preemption timer is
   then cleared in VM-exit handler, exit from vCPU thread is performed.

3) The guest is running, then hv_vcpu_run() is interrupted by
   hv_vcpu_interrupt() and vCPU thread quits.

4) vCPU thread has just made VM-exit, then exit_request is recorded and
   VMX-preemption timer is enabled but the exit request won't be
   satisfied until the next iteration of vCPU thread, no kick loss
   happens.

5) vCPU thread is far after hv_vcpu_run(), then exit_request is recorded
   and VMX-preemption timer is not enabled. The exit request will be
   satasfied on the next iteration of vCPU thread, like in 4). The kick
   is not lost.

6) If some external interupt happens we can satisify exit request and can
   clear VMX-preemption timer, i.e. kicks are coalesced with interrupts.

While at it, correct type of hvf_fd to the type of hv_vcpuid_t to avoid
compilation warnings.

1. https://opensource.apple.com/source/xnu/xnu-6153.81.5/osfmk/i386/mp.c

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---

Hi,

The version is just for review. There's an issue where interrupts
(x86_gsi_interrupt) don't reach HVF (i.e. no interrupts are loaded into
VMCS interruption info field). I've been triaging the issue for a while
and dtrace patches are byproduct of the triage efforts. Once I figure
out where interrupts are lost/suppressed (which won't happen until the
end of the next week due to vacation), I'll send v5 with kick patch and
the interrupt delivery fix.

Thanks,
Roman

Changes since v3:
 - Replaced TLS pthread_key_t with current_cpu (Paolo)
 - Eliminated race condition by signalling through atomic variables (Paolo)

Changes since v2:
 - Reworked workaround to minimize kick loss race. Use signals to
   interrupt vCPU thread outside of hv_vcpu_run() and turn-on/turn-off
   VMX-preemeption timer, while timer value is always zero. v3 also
   assumes that VMX-preemption timer control is always available on the
   hardware that supports HVF.

Changes since v1:
 - Reduced kick loss race (Paolo) and removed SIG_IPI blocking

 include/hw/core/cpu.h  |  2 +-
 include/sysemu/hvf.h   |  1 +
 softmmu/cpus.c | 13 ++---
 target/i386/cpu.h  |  1 +
 target/i386/hvf/hvf.c  | 60 --
 target/i386/hvf/vmcs.h |  1 +
 6 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 8f145733ce..deaf9fe7ca 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -454,7 +454,7 @@ struct CPUState {
 
 struct hax_vcpu_state *hax_vcpu;
 
-int hvf_fd;
+unsigned hvf_fd;
 
 /* track IOMMUs whose translations we've cached in the TCG TLB */
 GArray *iommu_notifiers;
diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index 6d3ee4fdb7..a1ab61403f 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -25,6 +25,7 @@ extern bool hvf_allowed;
 
 int hvf_init_vcpu(CPUState *);
 int hvf_vcpu_exec(CPUState *);
+void hvf_vcpu_kick(CPUState *);
 void hvf_cpu_synchronize_state(CPUState *);
 void hvf_cpu_synchronize_post_reset(CPUState *);
 void hvf_cpu_synchronize_post_init(CPUState *);
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index a802e899ab..a2c3ed93fe 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -1708,10 +1708,15 @@ static void qemu_cpu_kick_thread(CPUState *cpu)
 return;
 }
 cpu->thread_kicked = true;
-err = pthread_kill(cpu->thread->thread, SIG_IPI);
-if (err && err != ESRCH) {
-fprintf(stderr, "qemu:%s: %s", __func__, strerror(err));
-

Re: [RFC v3 2/8] cpus: prepare new CpusAccel cpu accelerator interface

2020-08-11 Thread Roman Bolshakov
On Mon, Aug 03, 2020 at 11:05:27AM +0200, Claudio Fontana wrote:
> The new interface starts unused, will start being used by the
> next patches.
> 
> It provides methods for each accelerator to start a vcpu, kick a vcpu,
> synchronize state, get cpu virtual clock and elapsed ticks.
> 
> Signed-off-by: Claudio Fontana 
> ---
>  hw/core/cpu.c  |   1 +
>  hw/i386/x86.c  |   2 +-
>  include/sysemu/cpu-timers.h|   9 +-
>  include/sysemu/cpus.h  |  36 
>  include/sysemu/hw_accel.h  |  69 ++-
>  softmmu/cpu-timers.c   |   9 +-
>  softmmu/cpus.c | 194 
> -
>  stubs/Makefile.objs|   2 +
>  stubs/cpu-synchronize-state.c  |  15 
>  stubs/cpus-get-virtual-clock.c |   8 ++
>  util/qemu-timer.c  |   8 +-
>  11 files changed, 231 insertions(+), 122 deletions(-)
>  create mode 100644 stubs/cpu-synchronize-state.c
>  create mode 100644 stubs/cpus-get-virtual-clock.c
> 
> diff --git a/hw/core/cpu.c b/hw/core/cpu.c
> index 594441a150..b389a312df 100644
> --- a/hw/core/cpu.c
> +++ b/hw/core/cpu.c
> @@ -33,6 +33,7 @@
>  #include "hw/qdev-properties.h"
>  #include "trace-root.h"
>  #include "qemu/plugin.h"
> +#include "sysemu/hw_accel.h"
>  
>  CPUInterruptHandler cpu_interrupt_handler;
>  
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 58cf2229d5..00c35bad7e 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -264,7 +264,7 @@ static long get_file_size(FILE *f)
>  /* TSC handling */
>  uint64_t cpu_get_tsc(CPUX86State *env)
>  {
> -return cpu_get_ticks();
> +return cpus_get_elapsed_ticks();

Hi Claudio,

I still don't understand why plural form of "cpus" is used in files,
CpusAccel interface name and cpus_ prefix of the functions/variables.

Original cpus.c had functions to create CPU threads for multiple
accelerators, that justified naming of cpus.c. It had TCG, KVM and other
kinds of vCPUs. After you factor cpus.c into separate implementations of
CPU interface it should get singular form.

I’m not a native English speaker but the naming looks confusing to me.

>  }
>  
>  /* IRQ handling */
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 54fdb2761c..bad6302ca3 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -87,7 +87,7 @@ bool cpu_is_stopped(CPUState *cpu)
>  return cpu->stopped || !runstate_is_running();
>  }
>  
> -static inline bool cpu_work_list_empty(CPUState *cpu)
> +bool cpu_work_list_empty(CPUState *cpu)
>  {
>  bool ret;
>  
> @@ -97,7 +97,7 @@ static inline bool cpu_work_list_empty(CPUState *cpu)
>  return ret;
>  }
>  
> -static bool cpu_thread_is_idle(CPUState *cpu)
> +bool cpu_thread_is_idle(CPUState *cpu)
>  {
>  if (cpu->stop || !cpu_work_list_empty(cpu)) {
>  return false;
> @@ -215,6 +215,11 @@ void hw_error(const char *fmt, ...)
>  abort();
>  }
>  
> +/*
> + * The chosen accelerator is supposed to register this.
> + */
> +static CpusAccel *cpus_accel;
> +
>  void cpu_synchronize_all_states(void)
>  {
>  CPUState *cpu;
> @@ -251,6 +256,102 @@ void cpu_synchronize_all_pre_loadvm(void)
>  }
>  }
>  
> +void cpu_synchronize_state(CPUState *cpu)
> +{
> +if (cpus_accel && cpus_accel->synchronize_state) {
> +cpus_accel->synchronize_state(cpu);

I think the condition can be removed altogether if you move it to the
bootom inside else body. cpu_interrupt_handler and cpu_interrupt() in
hw/core/cpu.c is an example of that. Likely cpu_interrupt_handler should
be part of the accel interface. You might also avoid indirected function
call by using standalone fuction pointer. Like that:


void cpu_synchronize_state(CPUState *cpu)
{
if (cpus_accel && cpus_accel->synchronize_state) {
cpus_accel->synchronize_state(cpu);
}
if (kvm_enabled()) {
kvm_cpu_synchronize_state(cpu);
}
else if (hax_enabled()) {
hax_cpu_synchronize_state(cpu);
}
else if (whpx_enabled()) {
whpx_cpu_synchronize_state(cpu);
} else {
cpu_synchronize_state_handler(cpu);
}
}

After you finish factoring, it becomes:


void cpu_synchronize_state(CPUState *cpu)
{
cpu_synchronize_state_handler(cpu);
}

cpu_register_accel would just assign non-NULL function pointer
from a CPUAccel field over generic_cpu_synchronize_state_handler.

Regards,
Roman

> +}
> +if (kvm_enabled()) {
> +kvm_cpu_synchronize_state(cpu);
> +}
> +if (hax_enabled()) {
> +hax_cpu_synchronize_state(cpu);
> +}
> +if (whpx_enabled()) {
> +whpx_cpu_synchronize_state(cpu);
> +}
> +}
> +



Re: [RFC v3 8/8] cpus: extract out hvf-specific code to target/i386/hvf/

2020-08-11 Thread Roman Bolshakov
On Mon, Aug 03, 2020 at 11:05:33AM +0200, Claudio Fontana wrote:
> register a "CpusAccel" interface for HVF as well.
> 
> Signed-off-by: Claudio Fontana 
> ---
>  softmmu/cpus.c|  63 
>  target/i386/hvf/Makefile.objs |   2 +-
>  target/i386/hvf/hvf-cpus.c| 131 
> ++
>  target/i386/hvf/hvf-cpus.h|  17 ++
>  target/i386/hvf/hvf.c |   3 +
>  5 files changed, 152 insertions(+), 64 deletions(-)
>  create mode 100644 target/i386/hvf/hvf-cpus.c
>  create mode 100644 target/i386/hvf/hvf-cpus.h
> 
> diff --git a/softmmu/cpus.c b/softmmu/cpus.c
> index 586b4acaab..d327b2685c 100644
> --- a/softmmu/cpus.c
> +++ b/softmmu/cpus.c
> @@ -33,7 +33,6 @@
>  #include "exec/gdbstub.h"
>  #include "sysemu/hw_accel.h"
>  #include "sysemu/kvm.h"
> -#include "sysemu/hvf.h"

I wonder if the declarations should be moved from sysemu/hvf.h to
someplace inside target/i386/hvf/:

int hvf_init_vcpu(CPUState *);
int hvf_vcpu_exec(CPUState *);
void hvf_cpu_synchronize_state(CPUState *);
void hvf_cpu_synchronize_post_reset(CPUState *);
void hvf_cpu_synchronize_post_init(CPUState *);
void hvf_cpu_synchronize_pre_loadvm(CPUState *);
void hvf_vcpu_destroy(CPUState *);

They're not used outside of target/i386/hvf/

I also wonder if we need stubs at all?

>  #include "exec/exec-all.h"
>  #include "qemu/thread.h"
>  #include "qemu/plugin.h"
> @@ -391,48 +390,6 @@ void qemu_wait_io_event(CPUState *cpu)
>  qemu_wait_io_event_common(cpu);
>  }
>  
> -/* The HVF-specific vCPU thread function. This one should only run when the 
> host
> - * CPU supports the VMX "unrestricted guest" feature. */
> -static void *qemu_hvf_cpu_thread_fn(void *arg)
> -{
> -CPUState *cpu = arg;
> -
> -int r;
> -
> -assert(hvf_enabled());
> -
> -rcu_register_thread();
> -
> -qemu_mutex_lock_iothread();
> -qemu_thread_get_self(cpu->thread);
> -
> -cpu->thread_id = qemu_get_thread_id();
> -cpu->can_do_io = 1;
> -current_cpu = cpu;
> -
> -hvf_init_vcpu(cpu);
> -
> -/* signal CPU creation */
> -cpu_thread_signal_created(cpu);
> -qemu_guest_random_seed_thread_part2(cpu->random_seed);
> -
> -do {
> -if (cpu_can_run(cpu)) {
> -r = hvf_vcpu_exec(cpu);
> -if (r == EXCP_DEBUG) {
> -cpu_handle_guest_debug(cpu);
> -}
> -}
> -qemu_wait_io_event(cpu);
> -} while (!cpu->unplug || cpu_can_run(cpu));
> -
> -hvf_vcpu_destroy(cpu);
> -cpu_thread_signal_destroyed(cpu);
> -qemu_mutex_unlock_iothread();
> -rcu_unregister_thread();
> -return NULL;
> -}
> -
>  void cpus_kick_thread(CPUState *cpu)
>  {
>  #ifndef _WIN32
> @@ -603,24 +560,6 @@ void cpu_remove_sync(CPUState *cpu)
>  qemu_mutex_lock_iothread();
>  }
>  
> -static void qemu_hvf_start_vcpu(CPUState *cpu)
> -{
> -char thread_name[VCPU_THREAD_NAME_SIZE];
> -
> -/* HVF currently does not support TCG, and only runs in
> - * unrestricted-guest mode. */
> -assert(hvf_enabled());
> -
> -cpu->thread = g_malloc0(sizeof(QemuThread));
> -cpu->halt_cond = g_malloc0(sizeof(QemuCond));
> -qemu_cond_init(cpu->halt_cond);
> -
> -snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/HVF",
> - cpu->cpu_index);
> -qemu_thread_create(cpu->thread, thread_name, qemu_hvf_cpu_thread_fn,
> -   cpu, QEMU_THREAD_JOINABLE);
> -}
> -
>  void cpus_register_accel(CpusAccel *ca)
>  {
>  assert(ca != NULL);
> @@ -648,8 +587,6 @@ void qemu_init_vcpu(CPUState *cpu)
>  if (cpus_accel) {
>  /* accelerator already implements the CpusAccel interface */
>  cpus_accel->create_vcpu_thread(cpu);
> -} else if (hvf_enabled()) {
> -qemu_hvf_start_vcpu(cpu);
>  } else {
>  assert(0);
>  }
> diff --git a/target/i386/hvf/Makefile.objs b/target/i386/hvf/Makefile.objs
> index 927b86bc67..af9f7dcfc1 100644
> --- a/target/i386/hvf/Makefile.objs
> +++ b/target/i386/hvf/Makefile.objs
> @@ -1,2 +1,2 @@
> -obj-y += hvf.o
> +obj-y += hvf.o hvf-cpus.o
>  obj-y += x86.o x86_cpuid.o x86_decode.o x86_descr.o x86_emu.o x86_flags.o 
> x86_mmu.o x86hvf.o x86_task.o
> diff --git a/target/i386/hvf/hvf-cpus.c b/target/i386/hvf/hvf-cpus.c
> new file mode 100644
> index 00..9540157f1e
> --- /dev/null
> +++ b/target/i386/hvf/hvf-cpus.c

I'd prefer singular form in variables and file names. More on that in
the comment to patch 2.

Besides that it works fine,

Reviewed-by: Roman Bolshakov 
Tested-by: Roman Bolshakov 

Regards,
Roman



  1   2   3   4   >