On Tue, 2019-06-18 at 10:35 +0000, Roman Kagan wrote: > On Tue, Jun 18, 2019 at 11:24:57AM +1000, Vadim Rozenfeld wrote: > > On Mon, 2019-06-17 at 14:49 -0300, Eduardo Habkost wrote: > > > On Mon, Jun 17, 2019 at 05:32:13PM +0000, Roman Kagan wrote: > > > > On Mon, Jun 17, 2019 at 11:23:01AM -0300, Eduardo Habkost > > > > wrote: > > > > > On Mon, Jun 17, 2019 at 01:48:59PM +0000, Roman Kagan wrote: > > > > > > On Sat, Jun 15, 2019 at 05:05:05PM -0300, Eduardo Habkost > > > > > > wrote: > > > > > > > The current default value for hv-spinlocks is 0xFFFFFFFF > > > > > > > (meaning > > > > > > > "never retry"). However, the value is stored as a signed > > > > > > > integer, making the getter of the hv-spinlocks QOM > > > > > > > property > > > > > > > return -1 instead of 0xFFFFFFFF. > > > > > > > > > > > > > > Fix this by changing the type of > > > > > > > X86CPU::hyperv_spinlock_attempts > > > > > > > to uint32_t. This has no visible effect to guest > > > > > > > operating > > > > > > > systems, affecting just the behavior of the QOM getter. > > > > > > > > > > > > > > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com> > > > > > > > --- > > > > > > > target/i386/cpu.h | 2 +- > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > Reviewed-by: Roman Kagan <rka...@virtuozzo.com> > > > > > > > > > > > > That said, it's tempting to just nuke qdev_prop_spinlocks > > > > > > and > > > > > > make > > > > > > hv-spinlocks a regular DEFINE_PROP_UINT32... > > > > > > > > > > Agreed. The only difference is that we would validate the > > > > > property at realize time instead of object_property_set(). > > > > > > > > Right. But currently it's validated to be no less than 0xfff > > > > and > > > > no > > > > bigger than 0xffffffff. The latter check would become > > > > unnecessary, > > > > and > > > > I'm unable to find any reason to do the former (neither spec > > > > references > > > > nor the log messages of the commits that introduced it). > > > > > > The 0xFFF lower limit was originally introduced by commit > > > 28f52cc04d34 ("hyper-v: introduce Hyper-V support > > > infrastructure"). > > > > > > Vadim, do you know where the 0xFFF limit comes from? > > > > I simply took this value from Windows Server 2008 R2 that > > I used as a reference while working on Hyper-V support for KVM. > > I also remember some paper (probably published by AMD ???) > > mentioned > > that 0x2fff seemed to have the best balance for PLE logic. > > The question is whether the user should be disallowed to set it below > 0xfff? > I don't see this mandated by the spec, so I'd rather remove the lower > limit and convert the property to a regular DEFINE_PROP_UINT32. >
Honestly, I don't have any strong opinions on this matter. Having some lower boundary limit seemed quite logical to me. However, if a user wants to experiment and see how the smaller number of spinlock acquire attempts before calling HvNotifyLongSpinWait will affect the overall system performance, then why not? Vadim. > Roman.