Re: [Xen-devel] [PATCH v11] x86/emulate: Send vm_event from emulate

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.09.2019 10:06, Alexandru Stefan ISAILA wrote:
> On 19.09.2019 16:59, Jan Beulich wrote:
>> Furthermore while you now restrict the check to linear address
>> based accesses, other than the description says (or at least
>> implies) you do not restrict it to read and exec accesses. It's
>> not clear to me whether that's intentional, yet it affects which
>> hvm_copy_*_linear() callers need auditing.
> 
> The pfec var is checked in hvm_monitor_check_p2m(). If you think it is 
> necessary I can add one more check here for (pfec & (PFEC_insn_fetch | 
> PFEC_write_access)).

hvm_monitor_check_p2m() gets called from two places, so a check
there won't help (afaict). The question whether to put an
additional check here depends on whether, as the description
says, you really only want to handle read/exec accesses here,
or whether you also want to cover write ones (in which case the
description should be adjusted so that it's not misleading).

>> Finally, what about ->arch.vm_event->send_event remaining set
>> after hvm_emulate_one_vm_event(), because hvm_monitor_check_p2m()
>> (the only place where the flag would get cleared) was never hit
>> in the process?
> Thanks for pointing this out, indeed it's a problem here. A solution can 
> be to move send_event = false; after hvm_emulate_one_vm_event() is 
> finished. And state in the comment that the user is in charge of 
> enabling and disabling the flag.
> Or just have it in both places.

For this aspect alone I think you want it in both places, but ...

>> And what about an instruction accessing two (or
>> more) distinct addresses? The flag would be clear after the first
>> one was checked afaict.
> 
> There is no problem here because emulation will stop after the first bad 
> access so there is no need to continue.

... for this moving it may indeed be necessary. I have to admit
that I don't follow your reply here: The flag will also be clear
after the first good access (afaict), and hence further accesses
by the same insn won't make it into hvm_monitor_check_p2m() at all.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel