Re: [Xen-devel] [PATCH RFC] x86/emulate: implement hvmemul_cmpxchg() with an actual CMPXCHG

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 03/29/2017 04:55 PM, Jan Beulich wrote:
>>>> On 28.03.17 at 12:50, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> On 03/28/2017 01:47 PM, Jan Beulich wrote:
>>>>>> On 28.03.17 at 12:27, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>> On 03/28/2017 01:03 PM, Jan Beulich wrote:
>>>>>>>> On 28.03.17 at 11:14, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>>>> I'm not sure that the RETRY model is what the guest OS expects. AFAIK, a
>>>>>> failed CMPXCHG should happen just once, with the proper registers and ZF
>>>>>> set. The guest surely expects neither that the instruction resume until
>>>>>> it succeeds, nor that some hidden loop goes on for an undeterminate
>>>>>> ammount of time until a CMPXCHG succeeds.
>>>>>
>>>>> The guest doesn't observe the CMPXCHG failing - RETRY leads to
>>>>> the instruction being restarted instead of completed.
>>>>
>>>> Indeed, but it works differently with hvm_emulate_one_vm_event() where
>>>> RETRY currently would have the instruction be re-executed (properly
>>>> re-executed, not just re-emulated) by the guest.
>>>
>>> Right - see my other reply to Andrew: The function likely would
>>> need to tell apart guest CMPXCHG uses from us using the insn to
>>> carry out the write by some other one. That may involve
>>> adjustments to the memory write logic in x86_emulate() itself, as
>>> the late failure of the comparison then would also need to be
>>> communicated back (via ZF clear) to the guest.
>>
>> Exactly, it would require quite some reworking of x86_emulate().
> 
> I had imagined it to be less intrusive (outside of x86_emulate()),
> but I've now learned why Andrew was able to get rid of
> X86EMUL_CMPXCHG_FAILED - the apparently intended behavior
> was never implemented. Attached a first take at it, which has
> seen smoke testing, but nothing more. The way it ends up being
> I don't think this can reasonably be considered for 4.9 at this
> point in time. (Also Cc-ing Tim for the shadow code changes,
> even if this isn't really a proper patch submission.)

I have this xenstored-related error when trying to build the latest
staging, not sure who this should be forwarded to (hopefully I'm not
spamming):

make -C xenstored install
make[6]: Entering directory `/home/red/work/xen.git/tools/ocaml/xenstored'
rm -f paths.ml.tmp;  printf "let %s = \"%s\";;\n" sbindir /usr/sbin
>>paths.ml.tmp;  printf "let %s = \"%s\";;\n" bindir /usr/bin
>>paths.ml.tmp;  printf "let %s = \"%s\";;\n" libexec /usr/lib/xen
>>paths.ml.tmp;  printf "let %s = \"%s\";;\n" libexec_bin
/usr/lib/xen/bin >>paths.ml.tmp;  printf "let %s = \"%s\";;\n" libdir
/usr/lib >>paths.ml.tmp;  printf "let %s = \"%s\";;\n" sharedir
/usr/share >>paths.ml.tmp;  printf "let %s = \"%s\";;\n" xenfirmwaredir
/usr/lib/xen/boot >>paths.ml.tmp;  printf "let %s = \"%s\";;\n"
xen_config_dir /etc/xen >>paths.ml.tmp;  printf "let %s = \"%s\";;\n"
xen_script_dir /etc/xen/scripts >>paths.ml.tmp;  printf "let %s =
\"%s\";;\n" xen_lock_dir /var/lock >>paths.ml.tmp;  printf "let %s =
\"%s\";;\n" xen_run_dir /var/run/xen >>paths.ml.tmp;  printf "let %s =
\"%s\";;\n" xen_paging_dir /var/lib/xen/xenpaging >>paths.ml.tmp;
printf "let %s = \"%s\";;\n" xen_dump_dir /var/lib/xen/dump
>>paths.ml.tmp;  printf "let %s = \"%s\";;\n" xen_log_dir /var/log/xen
>>paths.ml.tmp;  printf "let %s = \"%s\";;\n" xen_lib_dir /var/lib/xen
>>paths.ml.tmp;  printf "let %s = \"%s\";;\n" xen_run_stored
/var/run/xenstored >>paths.ml.tmp;  if ! cmp -s paths.ml.tmp paths.ml;
then mv -f paths.ml.tmp paths.ml; else rm -f paths.ml.tmp; fi
rm -f _paths.h.tmp;  echo "#define sbindir \"/usr/sbin\""
>>_paths.h.tmp;  echo "#define bindir \"/usr/bin\"" >>_paths.h.tmp;
echo "#define LIBEXEC \"/usr/lib/xen\"" >>_paths.h.tmp;  echo "#define
LIBEXEC_BIN \"/usr/lib/xen/bin\"" >>_paths.h.tmp;  echo "#define libdir
\"/usr/lib\"" >>_paths.h.tmp;  echo "#define SHAREDIR \"/usr/share\""
>>_paths.h.tmp;  echo "#define XENFIRMWAREDIR \"/usr/lib/xen/boot\""
>>_paths.h.tmp;  echo "#define XEN_CONFIG_DIR \"/etc/xen\""
>>_paths.h.tmp;  echo "#define XEN_SCRIPT_DIR \"/etc/xen/scripts\""
>>_paths.h.tmp;  echo "#define XEN_LOCK_DIR \"/var/lock\""
>>_paths.h.tmp;  echo "#define XEN_RUN_DIR \"/var/run/xen\""
>>_paths.h.tmp;  echo "#define XEN_PAGING_DIR
\"/var/lib/xen/xenpaging\"" >>_paths.h.tmp;  echo "#define XEN_DUMP_DIR
\"/var/lib/xen/dump\"" >>_paths.h.tmp;  echo "#define XEN_LOG_DIR
\"/var/log/xen\"" >>_paths.h.tmp;  echo "#define XEN_LIB_DIR
\"/var/lib/xen\"" >>_paths.h.tmp;  echo "#define XEN_RUN_STORED
\"/var/run/xenstored\"" >>_paths.h.tmp;  if ! cmp -s _paths.h.tmp
_paths.h; then mv -f _paths.h.tmp _paths.h; else rm -f _paths.h.tmp; fi
 MLOPT    store.cmx
File "store.ml", line 1:
Error: The files perms.cmi and define.cmi make inconsistent assumptions
       over interface Define
make[6]: *** [store.cmx] Error 2

This happens on "make dist".


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel